-
Notifications
You must be signed in to change notification settings - Fork 54
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
fix(ng-dev): force prompt should only ignore the failed validation #730
Conversation
Note: This also enables us to skip the "changes allow for target label" validation, fixing issues like we had last time. |
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.
I love the direction of this but had a question about how we are going about it.
1b5e9b3
to
3be2001
Compare
Currently when a validation check fails, the caretaker is prompted to continue forcibly if the failure was non-fatal. This seemed nice at first glance, but there is a real issue here. The force merge run might hide actual validations which might have not been hit before. To fix this, we introduce an incremental validation system where we run validations from a config (all by default) and upon failure, if non-fatal the individual validation can be ignored, re-attempting the merge. Also as part of this the validation logic has been refactored/reworked to individual files with some more complicated wrapper logic that ensures we properly skip and report validation failures in a safe way.
3be2001
to
c6654e4
Compare
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.
LGTM
…tion Do not query active release trains if target labeling is disabled
9aa5366
to
af4b7ad
Compare
…tion Cleanup remaining special flag references and remove unnecessary config read
…tion Improve error messaging styling
…tion Improve error messaging styling
This PR was merged into the repository by commit 0f1cace. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Currently when a validation check fails, the caretaker is prompted to
continue forcibly if the failure was non-fatal.
This seemed nice at first glance, but there is a real issue here. The
force merge run might hide actual validations which might have not been
hit before.
To fix this, we introduce an incremental validation system where we run
validations from a config (all by default) and upon failure, if
non-fatal the individual validation can be ignored, re-attempting the
merge.
Also as part of this the validation logic has been refactored/reworked
to individual files with some more complicated wrapper logic that
ensures we properly skip and report validation failures in a safe way.