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 :))
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]).
GitLab does provide this capability. They have a free CI thread even on the free accounts. GitLab is much better than GitHub.
Edit: well, maybe not exactly this functionality. It is CI based and not hook based. Still free CI thread is pretty awesome and can be used in this type of application.
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.
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.
It's been fantastic. People fear change, but after a short while, everyone was digging the benefits of commenting on other people's code and other people helping them make their code better.
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.
> 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.
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.
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.
Do what needs to be done at a given point in time to release. Just don't soothe your conscience by putting a TODO in the code. 99% that TODO will never get done, it's just clutter and code smell.
If your code is broken you're releasing something that's broken which is probably going to end badly. If the code is adequate then it's adequate.
Often it's just perception that doing it wrong is faster than doing it right. Prefer doing it right. (EDIT- doing it right is often faster)
I'm not sure this model works, in at least my case. Where there is perhaps some circuit you need to validate actually works with some sort of code - it's often useful to write some bad display driver for example so that the hardware team knows what needs to be changed and that process is not blocked.
The code may drive a very dirty clock signal that tells them:
1. Circuit works in the basic sense
2. Some part of the circuit doesn't work
3. Some other part of the circuit can now be added now that the basic circuit has been confirmed
It's okay saying "build the entire thing perfectly", but spending the difference between one and two weeks validating that something at least has the chance of working is massive. You can't afford to block every team that needs to know whether the part you are writing is even a valid concept.
This may come back down to design, but sometimes - especially with hardware, the manufacturer/API doesn't tell you the whole story and you need to know that as soon as possible to start compensating for it.
I think building a rough implementation and re-factoring it is way more agile than trying to build the correct version first time, possibly leaving some unknown problem to remain undiscovered for longer. It seems backwards to me when we're talking about anything that has an element of risk.
TODO messages are a terrible taxation on comprehension of code.
Most TODO items should be written "TODIDNT", what would we guess, 99% of TODO items never happen? What is the thinking here, "I knew I should have written the code more correctly, but I didn't -- commenting about what I failed to do here gives me a pass"?
Either do it, or don't. Don't tell everybody about what you didn't do, it reduces comprehension of the code at hand.
I'm a fan of a DESIGN.txt or other document that everyone can bitch and moan about shortcomings into. It's centralized and threaded by contributions, something like a wikimedia Talk page.
How do you reference a particular bit of code? For example, when you're writing the code you are aware that a number of lines could definitely do with improving but will get you out of the clear for the moment and past the milestone/deliverable. How do you specifically reference the code that you know is bad in this system?
Don't you find they get updated, moved or renamed? I personally have to deal with scope creep in my projects, so things often change from a strict design. But I imagine a longer project doing the same?
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."
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.
Background: I work mostly with projects that have <1 year delivery date.
It's always better to fix the code I completely agree - in reality there simply isn't always the time. In an idealistic world products aren't released until they are ready and bugs are never produced.
But... There's deadlines and there's certainly not time allocated to fixing code unless a bug appears there. Once a milestone or deliverable is met, as far as a manager is concerned it's case closed.
Whilst I agree with you from an idealistic stand point, for myself and my line of work I need to leave TODOs to fix the code if the time permits before the release time. Corners get cut and you need some way of knowing where those corners are.
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?
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.
That's horrible advice. The existing .git/hooks/pre-commit may not even be a shellscript, you could be appending to a binary file.
Even if it's a shellscript you can't just concatenate shellscripts together and expect them to work as thought you'd run them as two independent shellscripts.
Presumably, one's git hooks are not in danger of being lost to oblivion. If that's the case, neither bit of advice is ideal. If, however, one is careless enough that unix plumbing with obliterate the hook for eternity, then appending at least allows the original to exist some place.
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.
You could just add this pattern to something like https://github.com/landscapeio/dodgy -- Looks at Python code to search for things which look "dodgy" such as passwords or diffs"
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.
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.
46 comments
[ 2.5 ms ] story [ 104 ms ] thread[1] https://guides.github.com/introduction/flow/ [2] https://help.github.com/articles/enabling-required-status-ch... [3] https://developer.github.com/v3/repos/statuses/ [4] https://github.com/spring-projects/spring-boot
Edit: well, maybe not exactly this functionality. It is CI based and not hook based. Still free CI thread is pretty awesome and can be used in this type of application.
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.
It's been fantastic. People fear change, but after a short while, everyone was digging the benefits of commenting on other people's code and other people helping them make their code better.
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.
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.
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 ...
Do what needs to be done at a given point in time to release. Just don't soothe your conscience by putting a TODO in the code. 99% that TODO will never get done, it's just clutter and code smell.
If your code is broken you're releasing something that's broken which is probably going to end badly. If the code is adequate then it's adequate.
Often it's just perception that doing it wrong is faster than doing it right. Prefer doing it right. (EDIT- doing it right is often faster)
The code may drive a very dirty clock signal that tells them:
It's okay saying "build the entire thing perfectly", but spending the difference between one and two weeks validating that something at least has the chance of working is massive. You can't afford to block every team that needs to know whether the part you are writing is even a valid concept.This may come back down to design, but sometimes - especially with hardware, the manufacturer/API doesn't tell you the whole story and you need to know that as soon as possible to start compensating for it.
I think building a rough implementation and re-factoring it is way more agile than trying to build the correct version first time, possibly leaving some unknown problem to remain undiscovered for longer. It seems backwards to me when we're talking about anything that has an element of risk.
Most TODO items should be written "TODIDNT", what would we guess, 99% of TODO items never happen? What is the thinking here, "I knew I should have written the code more correctly, but I didn't -- commenting about what I failed to do here gives me a pass"?
Either do it, or don't. Don't tell everybody about what you didn't do, it reduces comprehension of the code at hand.
I'm a fan of a DESIGN.txt or other document that everyone can bitch and moan about shortcomings into. It's centralized and threaded by contributions, something like a wikimedia Talk page.
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.
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."
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.
It's always better to fix the code I completely agree - in reality there simply isn't always the time. In an idealistic world products aren't released until they are ready and bugs are never produced.
But... There's deadlines and there's certainly not time allocated to fixing code unless a bug appears there. Once a milestone or deliverable is met, as far as a manager is concerned it's case closed.
Whilst I agree with you from an idealistic stand point, for myself and my line of work I need to leave TODOs to fix the code if the time permits before the release time. Corners get cut and you need some way of knowing where those corners are.
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 seems to be an option, anyone have experience with that?
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.
It's wonderful.
Even if it's a shellscript you can't just concatenate shellscripts together and expect them to work as thought you'd run them as two independent shellscripts.
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.
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.
A relevant part is:
Full page http://jontelang.com/blog/2016/03/02/committing-with-sourcet...