
Show HN: Git Confirm. Git hook to catch accidentally committed code (TODO/Skip) - pimterry
https://github.com/pimterry/git-confirm
======
themckman
There isn't a lot that peeves me more than working with developers with poor
git commit habits. As someone who takes every commit they make seriously, it
can really demotivate/depress me when I look at my team's git log and see a
series of poorly thought out commits and messages. It's always enjoyable to
look at a piece of code, wonder why it is the way it is and to find that
answer in a well written commit message by the author of the change. If a tool
like his can at least help developers keep the diff portion of their commits
usable, then I hope more people consider using it.

It'd be interesting if it could learn from a project's history and have a
developer confirm that they really want to commit files that it has determined
to be unrelated, together.

~~~
nathancahill
Same. Last project I worked on, most people used git add ., git commit,
without ever looking at what was staged. The peeves were real. I'd love a tool
that didn't let you commit unrelated files.

~~~
jbverschoor
And that's how you find those lovely credentials on github.

------
bArray
I'm a regular at leaving "TODO" in the code. It's a reminder that the
implementation wasn't great and requires another look at some point. If
something breaks it's the first thing I fix in that related code. It's okay to
simply not have time to write every piece of code to perfection - it's
important to capture that fact though.

Another regular thing I use is "NOTE" for code that really is not obvious at
all. If my code doesn't read easily, other programmers (or my future self)
know to check it for in depth information.

If at the time of writing code you recognise it's difficult to understand or
not completely correct then I think it's the perfect time to capture that
information into the code.

~~~
YZF
What's going to happen is that TODO is going to:

a) stay in the code forever.

b) seen 5 years later where it doesn't even make sense any more and by a
different person.

If it's difficult to understand or not correct don't commit it. Just fix it.
It's a false economy. Friends don't let friends TODO ...

~~~
atjoslin
The reason most people tell me they use todos is, "you can't do everything up
front."

I agree, but I hate TODOs. They're the worst form of keeping track of
technical debt.

Just open a ticket! I like to have a tag/label called "debt" that I put these
"TODO, bad implementation" type of problems under.

That way at least it's tracked.

~~~
aiiane
A preferred method for doing this at Google is to open a bug, and then leave
the TODO in the code with a reference to the bug number. This has the
advantage that if someone comes by to refactor the code later, they'll have
the chance to go look at the bug (possibly taking it and working on it,
possibly resolving it as obsolete if the original issue no longer applies,
etc).

Having the two-directional link between the bug and the code is quite useful,
especially for what is probably the most frequent use I've seen for TODOs: "We
could do this better, but the better way is blocked on circumstances beyond
our control. Once those change, revisit this."

~~~
bArray
I actually like that - perhaps that allows you to implement blocking as well,
for example: "TODO: X cannot be improved until Z & Y is complete.".

The projects I work on tend to have a very quick life cycle, so there isn't
even a ticketing system in place other than the occasional post it note. There
just isn't the time and the team is tight.

------
boardwaalk
Honestly I think the best thing you can do towards making good commits is
having a good GUI client and using it well.

What I do is, after I'm done making a mess of things, I click through each
new/changed file and study the diff. I might need to commit or discard or edit
specific chunks, but at the end everything is staged. Then I can click through
the staged files quickly to check for mistakes while formulating a good commit
message.

(You could also use DIFF=vimdiff or something too)

I find looking at individual hunks and files and staging them piecewise makes
it much easier to maintain an attention to detail than doing a 'git diff'
where everything flows together and 'git commit -a' where you might miss
untracked or temporary files.

~~~
wingerlang
I agree, some time ago I wrote down why I liked GUIs and this was the main
point.

A relevant part is:

    
    
        This also means that for every commit, you ‘must’ skim
        through each file and stage only what you want to commit.
        This serves as a final reminder of your code and has on
        multiple occasions led me to moments where I had added
        some temp code which wasn’t supposed to be committed.
    

Full page [http://jontelang.com/blog/2016/03/02/committing-with-
sourcet...](http://jontelang.com/blog/2016/03/02/committing-with-
sourcetree.html)

------
web007

      curl https://.../hook.sh > .git/hooks/pre-commit

This seems like a bad idea, since you may already have a commit hook that this
would clobber.

Does anyone use any kind of plugin manager for git hooks? I know I can just
build a shellscript (and have done so extensively) but is there something like
pathogen / npm / bundler for a pluggable git hook architecture?

[https://github.com/brigade/overcommit](https://github.com/brigade/overcommit)
seems to be an option, anyone have experience with that?

~~~
spenuke
It's hard to completely generalize for the clobbering reason you point out.

Assuming that you essentially want to keep your hooks coupled with your source
code (or, at least, it's ok to do so), I've found that Make works pretty well
here.

In the root of the project directory, you have a subdirectory containing all
the hooks you need (or use a git submodule), and then you create a Make target
to copy those hooks (and make them executable) to your .git/hooks directory.
Make is nice because it won't clobber existing ones (unless they are older, in
which case you probably want them overwritten).

Then, whenever someone first clones the repo (or whenever you make a change to
the hooks), they just need to run make on that target again.

Obviously, not an ideal choice for everyone, but it's pretty simple and works
well for me.

------
Sir_Cmpwn
Git hooks are pretty useful for catching yourself. I recently put this in
.git/hooks/pre-commit for one of my repos:

    
    
        #!/usr/bin/env bash
        npm run lint

------
coherentpony
You can also git add -p, but that's a habit you have to develop.

~~~
pimterry
I built this mostly because I _really_ like git add -p, but afaict it doesn't
work at all for adding totally new files, and it's easy to miss things in the
middle of big chunks of changes. But yeah, that's exactly the inspiration.

~~~
abusque
> but afaict it doesn't work at all for adding totally new files

You can use git add --intent-to-add on your new file (or -N for short), and
then use git add -p as usual. You can then use the edit mode to remove hunks
you don't want to commit just yet.

Not quite as convenient as on an already tracked file, but it's reasonably
usable.

------
jdoliner
This looks pretty similar to `git commit -v` which brings up a diff of changes
in a text editor and prompts you for a commit message.

~~~
nilved
Those things are not really at all similar.

~~~
jdoliner
I guess they differ in that one interactively prompts you for each change and
one just shows you all the changes at once. But it's similar in that both are
a way to confirm that you're committing only what you want to commit.

------
john_oshea

        git config --add hooks.confirm.match "AKIA"
    

would probably be a good default for AWS users.

~~~
carterehsmith
For sure.

If I remember correctly, maybe a year ago, Github actually went through public
repos and emailed the people that had AKIAs in the repos. Apparently there
were many of them.

Myself, upon reading about that, I went through our (non-public) repo and,
sure enough, found like a dozen AKIAs with secret keys and all. Also found a
random AKIA in some binary file, false alarm.

But then I was like... wait a second. How about .pem files? Yup, found
several. .cer (some SSL certs), id_rsa? - yes to all.

That took a while to fix.

------
jquast
You could just add this pattern to something like
[https://github.com/landscapeio/dodgy](https://github.com/landscapeio/dodgy)
\-- Looks at Python code to search for things which look "dodgy" such as
passwords or diffs"

------
samstokes
I wish Github had a way of running these sort of checks before
accepting/rejecting a push. (I can understand why they would be reluctant to
run arbitrary user-supplied bash on their servers, though :))

~~~
praseodym
The GitHub flow [1] is to use pull requests for everything. Aside from being a
great code review tool, you can execute CI and other checks on them, even
making it required for those checks to pass before merging [2]. You can quite
easily hook your own checks [3], like the one from the original post or even a
check whether the committer signed a CLA (Spring Boot does this [4]).

[1]
[https://guides.github.com/introduction/flow/](https://guides.github.com/introduction/flow/)
[2] [https://help.github.com/articles/enabling-required-status-
ch...](https://help.github.com/articles/enabling-required-status-checks/) [3]
[https://developer.github.com/v3/repos/statuses/](https://developer.github.com/v3/repos/statuses/)
[4] [https://github.com/spring-projects/spring-
boot](https://github.com/spring-projects/spring-boot)

------
throwawayReply
It's a good idea but ultimately the people who don't care about this will
start doing: git commit -am "" /y

~~~
happyslobro
Don't forget -n (--no-verify). The last thing we need is some automated test
suite getting in our way ;)

------
dweinus
Does anyone know of anything similar as a Sublime package? I already do my
linting there, so this would be a great addition.

~~~
dweinus
In case anyone else is interested, this looks like a way to get there:
[http://www.sublimelinter.com/en/latest/creating_a_linter.htm...](http://www.sublimelinter.com/en/latest/creating_a_linter.html)

------
amitmerchant
This looks promising!

