Hacker News new | ask | show | jobs
by ath0 905 days ago
If you read nothing else, the commit message adding JIT support is worth your time: https://github.com/python/cpython/pull/113465
4 comments

To the guy who said the pull was horrible:

IMO, the most important thing about a pull request is to... actually be productive. I've worked with people in the past who would nit-pick my commit messages wanting me to waste hours of my time trying to use arcane Git commands. Any of which might and probably will clobber my work.

If you're doing Git reviews a good use of resources is to look for security problems, performance issues, and general engineering problems with someone's code, etc.

A BAD use of Git reviews is to spend the time making stylistic comments 'I would have written it like this, it looks much better', being overly pedantic about code formatting, or forcing your OCD on someone for their Git history. The reason people hate code reviews is somehow they're always done by this latter group of person.

If you're forcing people to waste time for silly reasons then you can count that they'll eventually leave your silly company. I know I have. I was once about to work at a company but saw that they literally used a committee of people to review every commit message wherein they would force every engineer to rewrite code for reasons that seemed to make astrology seem like a hard science. Consider not doing that.

A good reason for enforcing some rule on git commit messages though is to have a message that helps when bisecting to find where an issue was introduced in the past. If the commit message is too meanless or too different from whatever is the convention, it makes you spend more time when tracking and solving issues.
If you’ve a test checking the regression you can use that directly to find where the regression occurred without relying on the message, no?
No, the message should tell the why of the change, if you don't know you risk doing a fix that is simply removing the change and then you are back at the issue that motivate such change in first place. This is even harder if the person put the commit along others that are merged separately or worse, work directly on the main/master branch. I've been there a lot, it's not hard to write a good commit message, if you're in a project and everyone does their part except that one programmer, you will notice a lot every time a bug turns out to be introduced by that one person.
The reason one should write commit messages in the common form is to shave off a few seconds for every reviewer to immediately understand what is going on.

It's not unreasonable (within the limits of sanity, of course), it's just a matter of respecting other people's time.

If someone who made the changes can't explain things clearly, I'd even wager that they didn't understand what they're changing properly. A good commit message is needed as the author might be gone but software is maintained for decades. Productivity is easy, writing a good message isn't, as the latter seems to be hard for many and it's telling of their understanding of systems they change.
Many teams will just use squash merges to avoid wasting time on commit messages and end up with just one commit to the main branch and cleaner history.
When I'm having to wade back through previous commits trying to debug a problem, there's nothing I hate more than finding someone's large squashed commit. Instead of using automated bisect tooling, I have to try and understand what they did and untangle it. This might be an area of the codebase I'm unfamiliar with and it'll take even longer. Compared to this, writing a few extra commit messages costs nothing. I really don't get it.
That's like returning to publishing tarballs instead of having single, revertable commits.
`git rebase -i develop` is arcane? An editor opens where you put an 'r' in front of every commit that needs rewording. Save and close, and Git will successively open an editor for every commit message. Force Push and you're done.

If you're not willing to bother with command line: the Git client in Jetbrains also lets you edit commit messages in a very straightforward way.

Force push can be scary to some. Some nitpickers may also ask you to squash/reorder/split commits to satisfy their OCD.
I am one of those who would insist on correctly splitting commits. Commits are a communication tool, and a well-crafted series of commits makes reviewing pull requests, which is a chore for most people, a lot easier. Months or years later, it is still important to easily review changes.

In a Gitflow repository I don't particularly like squashing feature branches since a sequence of commits allows the author to better document what they were thinking about. Squashing is fine in Trunk-based development since feature branches are usually less extensive.

I hate intermediary merges on feature branches because they tend to make up a large portion of the branch history and are harder to review than normal commits.

Those nitpickers are probably people who have tried to track a regression in a big codebase before. It's not OCD, it's just useful version control.
Wow I underestimated how good it would be
Yeah, I'm getting old. I would prefer TLDR formal version and then the fun xmas version for people that feel festive. But hey I'm not paying for this so who am I to complain :)
400+ commits with a ton of them being "catch up to main"? Wow, I guess even good programmers don't know how to rebase/use git properly.