Skip to content

Conversation

@mwiebe
Copy link
Contributor

@mwiebe mwiebe commented Feb 14, 2025

What was the problem/requirement? (What/Why)

Improvements following up from #168,
and other cleanup/code duplication reduction.

What was the solution? (How)

  • Comments are more clear about difference between model parsing, when values may not be available, and job instantiation, when all values must be available for format strings.
  • Created a new file to hold general validator functions for some commonly used patterns.
  • Adjust the model to reduce the number of confusingly spurious errors, like when errors include "must be int" and "must be string" for the exact same field, seemingly contradictory errors.

What is the impact of this change?

Better experience with the code base and the error messages.

How was this change tested?

Ran the unit tests.

Was this change documented?

Code comments improved.

Is this a breaking change?

No.

Does this change impact security?

No.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@mwiebe mwiebe requested a review from a team as a code owner February 14, 2025 00:08
@mwiebe mwiebe force-pushed the comment-error-improvements branch 2 times, most recently from d0e36cf to e8accd2 Compare February 14, 2025 00:22
Copy link

@leongdl leongdl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome refactoring!

leongdl
leongdl previously approved these changes Feb 14, 2025
jusiskin
jusiskin previously approved these changes Feb 14, 2025
* Comments are more clear about difference between model parsing, when
  values may not be available, and job instantiation, when all values
  must be available for format strings.
* Created a new file to hold general validator functions for some
  commonly used patterns.
* Adjust the model to reduce the number of confusingly spurious errors,
  like when errors include "must be int" and "must be string" for the
  exact same field, seemingly contradictory errors.

Signed-off-by: Mark Wiebe <[email protected]>
@mwiebe mwiebe dismissed stale reviews from jusiskin and leongdl via 41c4bfb February 14, 2025 18:02
@mwiebe mwiebe force-pushed the comment-error-improvements branch from e8accd2 to 41c4bfb Compare February 14, 2025 18:02
@mwiebe mwiebe changed the title chore: Refactor to improve comments and reduce code duplication refactor: Refactor to improve comments and reduce code duplication Feb 14, 2025
@sonarqubecloud
Copy link

@mwiebe mwiebe merged commit a825060 into OpenJobDescription:mainline Feb 14, 2025
19 checks passed
@mwiebe mwiebe deleted the comment-error-improvements branch February 14, 2025 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants