Add option to fail on error when running with upload-to-github #327
Conversation
👷 Deploy request for squawkhq pending review.Visit the deploys page to approve it
|
wmartins
force-pushed
the
issue/326/add-option-to-exit-on-error
branch
from
7355e20 to
e7a76b0
Compare
cli/src/subcommand.rs
Outdated
| @@ -167,5 +174,10 @@ pub fn check_and_comment_on_pr( | |||
| &comment_body, | |||
| )?; | |||
| } | |||
|
|
|||
| if !is_empty && exit_on_error { | |||
There was a problem hiding this comment.
isn't !is_empty always true because we'll return on line 143 if is_empty is true?
| if !is_empty && exit_on_error { | |
| if exit_on_error { |
There was a problem hiding this comment.
You're absolutely right! Done in e5de367.
cli/src/subcommand.rs
Outdated
| &gh, | ||
| &github_repo_owner, | ||
| &github_repo_name, | ||
| github_pr_number, | ||
| &comment_body, | ||
| )?); | ||
| )?; | ||
| } | ||
| } | ||
| if let Some(github_token) = github_token { |
There was a problem hiding this comment.
| if let Some(github_token) = github_token { | |
| else if let Some(github_token) = github_token { |
so we don't have a case where we could have commont_on_pr run twice
There was a problem hiding this comment.
I missed that, thanks for mentioning. Done in fcc2dca.
cli/src/subcommand.rs
Outdated
| @@ -59,6 +61,9 @@ pub enum Command { | |||
| /// --exclude=require-concurrent-index-creation,ban-drop-database | |||
| #[structopt(short, long, use_delimiter = true)] | |||
| exclude: Option<Vec<RuleViolationKind>>, | |||
| /// Exits with an error code when specified | |||
| #[structopt(long)] | |||
| exit_on_error: bool, | |||
There was a problem hiding this comment.
thoughts on calling it something like:
| exit_on_error: bool, | |
| exit_on_violations: bool, |
or maybe
| exit_on_error: bool, | |
| exit_with_code_on_violations: bool, |
since we do exit with an error already, just not when there's violations?
There was a problem hiding this comment.
Which one do you prefer? Another option on the table would be instead of using exit using something like fail: fail_on_violations
|
Looks good, added a few minor comments! |
Example error: ``` Upload to GitHub failed: Found 6 violation(s) across 1 file(s) ```
|
@sbdchd I made some changes. I think this is good to merge |
This PR potentially fixes #326 and adds an option to fail on error when running
upload-to-github.Disclaimer: I'm not well versed in Rust, so feel free to suggest any modifications.
Also, if you don't agree with the solution for the problem itself, I don't mind closing this one and the issue.
Thank you!