Skip to content
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

Reorganize validation message level determination as a precursor to single rule disjunction #1366

Merged
merged 26 commits into from
Mar 21, 2024

Conversation

GiaJordan
Copy link
Contributor

@GiaJordan GiaJordan commented Feb 8, 2024

This PR will help enable single rule disjunction by reworking how validation message levels are determined and the IsNA rule modifier introduced in #1200. The error/warning/no-message determination logic of all validation rules has been reworked to be compatible with this new rule modifier and has been simplified to be easier to maintain.

Rule compatibility with the IsNA modifier will be finished in a subsequent PR.

@GiaJordan GiaJordan marked this pull request as draft February 8, 2024 22:34
@GiaJordan GiaJordan changed the title Develop val error handling fds 1352 Reorganize validation message level determination Feb 8, 2024
@GiaJordan GiaJordan changed the title Reorganize validation message level determination Feature: Add single rule disjunction Feb 12, 2024
@GiaJordan GiaJordan changed the title Feature: Add single rule disjunction Feature: Enable single rule disjunction Feb 12, 2024
@GiaJordan GiaJordan changed the title Feature: Enable single rule disjunction Reorganize validation message level determination as a precursor to single rule disjunction Feb 14, 2024
@GiaJordan GiaJordan marked this pull request as ready for review February 15, 2024 18:04
@GiaJordan
Copy link
Contributor Author

@mialy-defelice resolving the conflicts was trivial; this is ready to be reviewed

@mialy-defelice
Copy link
Contributor

@GiaJordan I see how to the functions were refactored for simplicity, but I am having trouble following how this enables single rule disjunction? Can you explain this again?

@GiaJordan
Copy link
Contributor Author

@mialy-defelice single rule disjunction will be enabled with #1369
I separated the refactor from the feature addition for better organization and ease of review

@mialy-defelice
Copy link
Contributor

@GiaJordan I guess my question came from this part of the description:

 The error/warning/no-message determination logic of all validation rules has been reworked to be compatible with this new rule modifier

How do these changes allow compatibility?

@GiaJordan
Copy link
Contributor Author

How do these changes allow compatibility?

@mialy-defelice When the IsNA rule was introduced, it was introduced only for the type checking rules because it was specific to @/andrewelamb's use case then.

The logic for IsNA now is in get_message_level() which is used by all of the rules when determining which level of message to raise.

@mialy-defelice
Copy link
Contributor

Ah gotcha, I understand now, thanks!

@GiaJordan
Copy link
Contributor Author

@mialy-defelice no problem!

Copy link

sonarqubecloud bot commented Mar 7, 2024

Quality Gate Passed Quality Gate passed

Issues
14 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
12.0% Duplication on New Code

See analysis details on SonarCloud

@mialy-defelice
Copy link
Contributor

Pushing up to dev, then will merge single rule disjunction directly to dev.

@mialy-defelice mialy-defelice merged commit e944103 into develop Mar 21, 2024
4 checks passed
@mialy-defelice mialy-defelice deleted the develop-val-error-handling-FDS-1352 branch March 21, 2024 16:58
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.

2 participants