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

Add -F/--fail-fast option #1089

Merged
merged 1 commit into from
May 20, 2014
Merged

Add -F/--fail-fast option #1089

merged 1 commit into from
May 20, 2014

Conversation

jonas054
Copy link
Collaborator

This is something that I have found useful when working with correcting offenses (in RuboCop itself). I have an external program that monitors file changes in the directory tree and re-runs ./bin/rubocop when something changes. When I save a file it is the first one checked in the next run, and execution stops at the first file with offenses.

There's some overlap of functionality between guard-rubocop and this option, but I just found that adding this functionality inside RuboCop itself solved my problem. I'd like to hear what @yujinakayama thinks, and @bbatsov of course.

@bbatsov
Copy link
Collaborator

bbatsov commented May 15, 2014

Seems like a good idea to me. The code looks fine.

@yujinakayama
Copy link
Collaborator

Initially I felt this feature is not efficient since running rubocop on the entire project for each modification of a file is wasteful. But I'm satisfied after reading the implementation that sorts files in modification time order. So I'm 👍 for this feature. I think it would be better if the description includes a note about the file inspection order.

@bbatsov
Copy link
Collaborator

bbatsov commented May 18, 2014

@jonas054 Let me know when you've updated the description as suggested by @yujinakayama.

@mikegee
Copy link
Contributor

mikegee commented May 18, 2014

I don't understand why the description would be improved with a note about the inspection order. Wouldn't this feature work exactly the same without that sorting (perhaps a bit slower)? It seems like an implementation detail to me.

@bbatsov
Copy link
Collaborator

bbatsov commented May 18, 2014

If people didn't know the feature is modification time aware, they might not be inclined to use it as often as they would otherwise (or at least I think so).

@jonas054
Copy link
Collaborator Author

@bbatsov I'll try to do the update soon (--help text, README, and also CHANGELOG).

@mikegee I agree with @yujinakayama and @bbatsov that describing inspection order is a good idea, and that people are more likely to try the feature if they get a glimpse into this rather important implementation detail. It's called "fail fast", so failing as quickly as possible is key.

Inspect files in order of modification time, newest first. And stop after the first file with offenses.
@jonas054
Copy link
Collaborator Author

Oops, made a strange self-referencing comment for this PR. Last one should be OK.

bbatsov added a commit that referenced this pull request May 20, 2014
@bbatsov bbatsov merged commit 7dd2b13 into rubocop:master May 20, 2014
@bbatsov
Copy link
Collaborator

bbatsov commented May 20, 2014

👍

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

Successfully merging this pull request may close these issues.

4 participants