-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ✨ add simple helper functions #962
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Luke W. Johnston <[email protected]>
Co-authored-by: Signe Kirk Brødbæk <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still loving the smaller PRs 👌 🌟
seedcase_sprout/core/sprout_checks/check_resource_id_in_data_path.py
Outdated
Show resolved
Hide resolved
tests/core/sprout_checks/test_check_resource_id_in_data_path.py
Outdated
Show resolved
Hide resolved
tests/core/sprout_checks/test_check_resource_id_in_data_path.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Signe Kirk Brødbæk <[email protected]>
…checks-2-simple-helper-functions
tests/core/sprout_checks/test_check_resource_id_in_data_path.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Signe Kirk Brødbæk <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌟
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Just some small comments and suggestions 😁
seedcase_sprout/core/sprout_checks/check_resource_id_in_data_path.py
Outdated
Show resolved
Hide resolved
seedcase_sprout/core/sprout_checks/check_resource_id_in_data_path.py
Outdated
Show resolved
Hide resolved
@@ -3,8 +3,10 @@ | |||
get_json_path_to_resource_field, | |||
) | |||
|
|||
CHECKS_TYPE_ERROR_MESSAGE = "{field_value} is not of type '{field_type}'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could arguably go into a separate file or even be exposed by the checks
package. As #978 will rework this, I'll just leave it like this for now.
Description
This PR adds some simple helper functions that will be used when running more complex checks in Sprout.
As a rule, I tried to write simple checks in such a way that they only check a single thing. So if a function checks whether a field is blank, it won't fail if the field is missing or is of the wrong type, because there are separate checks for that. That said, it's always possible to argue that this doesn't make sense for some functions -- so feel free to argue if you think any checks should behave differently!
This PR needs an in-depth review.
Checklist
just run-all