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

Return a non-zero exit code when matches exist #37

Closed
wants to merge 1 commit into from
Closed

Return a non-zero exit code when matches exist #37

wants to merge 1 commit into from

Conversation

guilhermesimoes
Copy link

This is just to get the conversation started around some code to solve #34.

Just as an example, rails_best_practices returns the number of errors found as the exit code. But reek doesn't.
I don't feel strongly one way or the other. But if you're not using the exit codes for anything else, why not? It could be useful.

Also, should this really need a flag to work? Shouldn't this just be the default behavior of the executable?

@guilhermesimoes guilhermesimoes changed the title Return the number of matches as the exit code of the executable Return a non-zero exit code when matches are found Jul 11, 2014
@guilhermesimoes guilhermesimoes changed the title Return a non-zero exit code when matches are found Return a non-zero exit code when matches exist Jul 11, 2014
@zenspider
Copy link
Member

I'm all for the idea, but I hesitate to make it exit equal to the size / score / whatever. Some code bases are horrific. For example, ruby_parser currently scores 15589. :)

@zenspider
Copy link
Member

That said, I'm not sure having a non-empty flay report is an error.

@guilhermesimoes
Copy link
Author

Score of 15589? Ouch :P

I don't think a non-empty Flay report is an error either. However, non-zero exit codes can carry special meanings. In this case, it would be mostly useful in continuous integration to signal that some code duplication was found.

Though I must say, there is always some code duplication reported by Flay that you can't (or aren't supposed to) fix, like Rails' RESTful controllers. So if Flay's exit codes were either just 0 or 1, I believe they would be of limited use since Flay would return 1 most of the time.

@zenspider zenspider self-assigned this Aug 18, 2014
@zenspider
Copy link
Member

The more I think about this, the less I like it. Unix exit codes are normally reserved for reporting success or error conditions. Having a flay score is not an error. Not being able to parse something is an error. I'd rather flay report 1 if it can't parse than because there is code that should be reviewed and then potentially accepted as-is.

For example, you'd never want to see a team set up the following nightly cronjob:

flay lib &> flay.txt || mail -s "error in flay" team < flay.txt

It would simply be noise. Nor would you want a CI to report a failure based on a non-zero exit from flay.

As such, I don't see this really conveying the right type of information via exit code.

@zenspider zenspider closed this Aug 29, 2014
@guilhermesimoes
Copy link
Author

I can see your point.

I think you should also close #34 then.

@seattlerb seattlerb locked and limited conversation to collaborators Jan 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants