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

Using rubocop with -a option. #214

Closed
skywinder opened this issue May 19, 2015 · 8 comments
Closed

Using rubocop with -a option. #214

skywinder opened this issue May 19, 2015 · 8 comments
Labels

Comments

@skywinder
Copy link

I try to use rubocop with -a (--auto-correct) option to fix all minor issues automatically.

I try to put in the .overcommit.yml file strings like

PreCommit:
  RuboCop:
    enabled: true
    command: ['rubocop', '-a'] 
    #or this:
    #command: ['bundle', 'exec', 'rubocop', '-a'] 
    on_warn: fail # Treat all warnings as failures

But it's not apply fixes to files before commit.

So, questions is:
How to run rubocop -a before pre-commit checks automatically? Is there any option for it in overcommit project?

@jawshooah
Copy link
Collaborator

You'll have to write a custom hook to accomplish this, since the built-in RuboCop hook is intended simply to report errors, not to alter files before commit.

@skywinder
Copy link
Author

@jawshooah yes, but i send another parameter to rubocop command (as you can see in example above), that run with -a --auto-correct option should also fix warnings. What I missed?

@jawshooah
Copy link
Collaborator

The core problem is that any changes rubocop --auto-correct makes are never staged. Overcommit sets up the hook environment by stashing all changes but keeping the index, runs the hooks, then restores the environment by doing a hard reset and re-applying the stash.

Actually, I'm not sure that you could even accomplish what you want with a custom hook at all, since restoring the environment wipes out any changes made during the hook run. @sds, can you think of a way around this?

@sds sds added the question label May 20, 2015
@sds
Copy link
Owner

sds commented May 20, 2015

@skywinder Overcommit is not intended to run hooks that modify any files or add/remove files from the git index. It's not really an intended use case due to technical details of how we implement the pre-commit hook saving and restoring of the working tree.

It also keeps the tool open to supporting cool features in future like running hooks in parallel (#144) — the moment you start to encourage hooks to modify files you provide more ways for a user to shoot themselves in the foot since you have multiple actors (hooks) modifying a shared resource (the git index).

My recommendation is your custom hook should simply print out the command that should be run to auto correct the offenses if it finds any. Hope that helps.

@sds sds closed this as completed May 20, 2015
@skywinder
Copy link
Author

@sds thanks for clarifying! It's definitely makes sense!

@janwerkhoven
Copy link

Sorry that's not helpful at all. The rubocop --auto-correct command is one of the most useful of them all. Nobody wants to dive in and indent, remove spaces and refactor syntax manually. Rubocop does this excellently FOR YOU. Overcommit should allow such commands.

@jawshooah
Copy link
Collaborator

@janwerkhoven What @sds explained above is that preserving file modifications during hook runs would require a significant overhaul of Overcommit's internals, and it just isn't worth it to support this one use-case. Especially when you can accomplish the same thing by just running rubocop --auto-correct yourself when Overcommit notifies you of errors on commit.

@sds
Copy link
Owner

sds commented Jan 19, 2016

We're tracking adding support for pre-commit hooks with file-changing side effects in #238. It's definitely something we know will be useful. Open to well-tested pull requests for those who want it sooner rather than later!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants