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

CodeClimate engine - exit code #790

Closed
andyw8 opened this issue Dec 20, 2015 · 7 comments
Closed

CodeClimate engine - exit code #790

andyw8 opened this issue Dec 20, 2015 · 7 comments
Assignees

Comments

@andyw8
Copy link
Collaborator

andyw8 commented Dec 20, 2015

Currently, Reek exits with a status of 2 when it detect smells.

When running Reek as a Code Climate engine (see #763), the engine needs to exit with a code of 0 when there are smells.

I'm trying to think how this can be elegantly handled in Reek.

The most direct way would be to add a conditional in Application#report_smells which reads the options.report_format, but that's essentially a type check smell.

I was thinking a better approach might be to set the status codes on a per-formatter basis, so each report would have methods for #success_exit_code, #error_exit_code and #smells_exit_code.

This has the advantage that any future engines can control the exit codes, but it's perhaps a premature optimisation since this is currently only needed for Code Climate.

Any thoughts?

@andyw8
Copy link
Collaborator Author

andyw8 commented Dec 20, 2015

(Another option could be a command-line override, similar to RSpec's --failure-exit-code).

@troessner
Copy link
Owner

I was thinking a better approach might be to set the status codes on a per-formatter basis, so each report would have methods for #success_exit_code, #error_exit_code and #smells_exit_code.

I like that, nice and clean!
@mvz && @chastell wdyt?
Who can look into this? If nobody can take a stab I can go for that over xmas.

@mvz
Copy link
Collaborator

mvz commented Dec 21, 2015

Let me ponder this, I have some ideas but I cannot elaborate on them right now.

@mvz
Copy link
Collaborator

mvz commented Dec 28, 2015

I would go for the simplest implementation possible right now.

Since this whole engine is basically meant to run in a Docker instance, we may want to eventually create a separate executable for it that is only included in the Docker image and not in the gem, thus removing the dependency on code climate specific code and for special exit code handling in the main gem. (I have zero Docker experience so take this with a grain of salt.)

@troessner
Copy link
Owner

Taking this over after our mail discussion.

I think I'll go for @andyw8 suggestion

(Another option could be a command-line override, similar to RSpec's --failure-exit-code).

which I find really neat.

@troessner troessner self-assigned this Dec 29, 2015
@troessner
Copy link
Owner

To quote the CodeClimate spec:

An engine must exit with a zero exit code to be considered a success. Any nonzero exit code indicates a fatal error in the static analysis and the results for the entire analysis will be discarded (even if some were previously emitted).

-> wip

@troessner
Copy link
Owner

Fix up in #808

@mvz mvz closed this as completed in #808 Jan 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants