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

Doesn't exit with non-zero error code on error by default #1083

Closed
follmann opened this Issue Aug 25, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@follmann

follmann commented Aug 25, 2017

Hi, I was wondering why brakeman doesn't exit with a non-zero exit code on errors by default. The documentation actually suggests that it should (brakeman options on the -z flag):

Brakeman returns 0 as an exit code unless something went very wrong. To return an error code when warnings were found

Only when you use the recently introduced --exit-on-error it does, I thought very wrong means an error 😸

Also vote for the -z flag as default (see #852)

Tested and verified on both v3.7.2 and v3.2.1

Thanks!

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Aug 26, 2017

Owner

"very wrong" was intended to mean "errors from which Brakeman does not recover."

I don't have a strong feeling on this one way or another, so I guess it can be the default to match --exit-on-warn.

Owner

presidentbeef commented Aug 26, 2017

"very wrong" was intended to mean "errors from which Brakeman does not recover."

I don't have a strong feeling on this one way or another, so I guess it can be the default to match --exit-on-warn.

@follmann

This comment has been minimized.

Show comment
Hide comment
@follmann

follmann Aug 27, 2017

Ah I see!

I think for all uses in scripts that rely on a meaningful exit code (which I guess is a 100% match e.g. in continuous integration setups) it would be the better default. People that just run brakeman to see the report won't be negatively affected by the change either.

follmann commented Aug 27, 2017

Ah I see!

I think for all uses in scripts that rely on a meaningful exit code (which I guess is a 100% match e.g. in continuous integration setups) it would be the better default. People that just run brakeman to see the report won't be negatively affected by the change either.

presidentbeef added a commit that referenced this issue Aug 30, 2017

presidentbeef added a commit that referenced this issue Aug 30, 2017

Repository owner locked and limited conversation to collaborators Sep 25, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.