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

abort if there are unstaged changed files #445

Closed
sfahlberg opened this issue Oct 18, 2016 · 8 comments
Closed

abort if there are unstaged changed files #445

sfahlberg opened this issue Oct 18, 2016 · 8 comments
Labels

Comments

@sfahlberg
Copy link

Hi there,
I am looking for a way to abort overcommit's checks if there are unstaged commits. Is there any functionality currently available that will do that?

@jawshooah
Copy link
Collaborator

jawshooah commented Oct 18, 2016

May I ask why? Overcommit stashes unstaged changes before running pre-commit hooks, so they shouldn't be having any effect on them.

@sfahlberg
Copy link
Author

@jawshooah it's functionality we currently have in our commit hooks. What if we just wanted a warning if you had unstaged commits?

Does it stash pop the changes once the hooks run?

@sds sds added the question label Oct 18, 2016
@sds
Copy link
Owner

sds commented Oct 18, 2016

@sfahlberg yes, the stashed changes are popped so that your working tree is restored to its previous state.

Also, to clarify, you meant "unstaged changes", not "unstaged commits," right? There's no such thing as an "unstaged" commit unless I'm missing something (just want to make sure I understand the request).

Pre-commit hooks will only run against changes which have been added to the index. Unless you have some sort of work flow where you explicitly want to prevent your developers committing their staged changes while there are unstaged changes to tracked files, Overcommit's method of stashing your unstaged changes before running pre-commit hooks should suffice here.

@sfahlberg
Copy link
Author

yeah, I meant unstaged changes. Good to know about the stashing.

Is there a way to display a warning that there are unstaged changes? @sds

@jawshooah
Copy link
Collaborator

@sfahlberg I think that's a bit outside the scope of Overcommit. It's not uncommon at all to have unstaged changes when you commit, and there's nothing inherently dangerous about it, so it would be strange for us to warn about it.

Ideally you should be using a command prompt that reports the status of the repo you're in after every command, e.g. bash-git-prompt.

@sds
Copy link
Owner

sds commented Oct 18, 2016

It's not currently possible to warn if there are unstaged changes. I agree with @jawshooah that this is also bit odd to enforce. I personally work within a repo with unstaged changes all the time. I have a hard time seeing this being useful rather than just being noisy, but your development process may just be very different. In general, my experience tells me that hooks that "warn" are ignored and your developers will not see much value from such a hook.

@themattman
Copy link

themattman commented Nov 3, 2016

There are two types of "unstaged changes":

  1. A file is modified and not staged
  2. A file is modified, staged, and then modified again. So there exists staged and unstaged changes for that file.

A visual representation of these two cases:

$ git status -s
 M file1
MM file2

If a hook makes modifications to files, the git-stash approach will break for case 2 above. For a hook to make changes to a file, it must then git-add the file after making changes. I haven't been able to find a way to git-add specific lines of a file programmatically. It's all-or-none when scripting git-add.

I'm not familiar with Overcommit enough to know whether hooks modify files. If hooks never make changes to the underlying staged files, then you can ignore my comment :)

Edit: Ah, I see that hooks that modify files is not supported re: #238. This is an important consideration when thinking about adding that feature.

@sds
Copy link
Owner

sds commented Nov 16, 2016

If a hook makes modifications to files, the git-stash approach will break for case 2 above.

Yes, this is an important point, @themattman, and it's one of the reasons why #238 has not been implemented. To get that fully right would take some very careful planning and consideration, requiring time which I personally don't have at the moment.

Very open to a well-tested pull request that addresses #238. I'm closing this issue in favor of that one. Thanks for your feedback!

@sds sds closed this as completed Nov 16, 2016
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