-
Notifications
You must be signed in to change notification settings - Fork 85
Extract non-default-branch and modifies-submodules warnings out of the assign
handler
#1922
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
Conversation
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.
Thank you! Looks good, left a few nits.
let mut warnings = Vec::new(); | ||
|
||
if let Some(assign_config) = &config.assign { | ||
// For legacy reasons the non-default-branch and modifies-submodule warnings |
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.
We might want to eventually introduce a [check_commits]
config option, but that doesn't need to happen in this PR.
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.
Agree, I think it make sense to have a [check_commits]
config for new warnings.
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.
Thanks! I tried it on my test repo and it seems to work fine. I will try to implement some end-to-end tests for this later, as it looks rather simple.
808c8c2
to
4c602da
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.
Great, thank you!
This PR is the first step of #1901 (comment)
It extracts non-default-branch and modifies-submodules warnings out of the
assign
handler and creates acheck_commits
handler.r? @Kobzol