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

Pre commit hooks: Should stash changes before checking #8

Open
windaishi opened this issue Sep 22, 2017 · 4 comments
Open

Pre commit hooks: Should stash changes before checking #8

windaishi opened this issue Sep 22, 2017 · 4 comments
Labels
bug Issues that describe an unexpected behaviour in an existing functionality. minor "bug" issues that are neither "critical" nor "major".

Comments

@windaishi
Copy link
Contributor

There is a small problem with the pre commit hooks that happens pretty often to me.
When the commit failes because of a style error in the code and you fix the styles afterwars but forget to use "git add" for this files, the pre commit hook does NOT fail and the files with the code style errors are commited.
I suggest that before every code style check, all changed to files that are not added to commit, should be stashed first. After the code style stash should be poped again.

@windaishi windaishi added the bug Issues that describe an unexpected behaviour in an existing functionality. label Sep 22, 2017
@fixpunkt fixpunkt added major "bug" issues that completely break a business process, with no reasonable workaround available. minor "bug" issues that are neither "critical" nor "major". and removed major "bug" issues that completely break a business process, with no reasonable workaround available. labels Sep 22, 2017
@fixpunkt
Copy link
Member

Yup, that's one option. But I also suggest you get in the habit of using git commit --all instead of adding and then committing.

@samuelvogel
Copy link
Member

I do actually propose to individually add all changes while simultaneously reviewing them and deciding what should actually be part of the commit (but here is not the right place to discuss this 😉).

@thomasnordquist
Copy link
Contributor

thomasnordquist commented Sep 22, 2017

To prevent this to occur, we could add a "dirty" check in our pre-push hook.
"dirty repo" => repo has unstaged changes

I would propose to use git diff --shortstat to list changes in managed changes (so that unmanaged files won't mark a repo as dirty):
https://stackoverflow.com/questions/2657935/checking-for-a-dirty-index-or-untracked-files-with-git

Example for pre-push hook:

if [ "`git diff --shortstat`" != "" ]; then
  echo "The current branch is dirty, commit/stash all your changes before git push";
  exit 1;
fi;

Edit: replaced pre-commit with pre-push

@fixpunkt
Copy link
Member

fixpunkt commented Sep 22, 2017

I do actually propose to individually add all changes while simultaneously reviewing them and deciding what should actually be part of the commit (but here is not the right place to discuss this 😉).

I review changes before running git commit -a and then review the diff shown in the commit editor again...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues that describe an unexpected behaviour in an existing functionality. minor "bug" issues that are neither "critical" nor "major".
Projects
None yet
Development

No branches or pull requests

4 participants