Hacker News Reader

Comment by 5350-uiop-1130 | Hacker News Reader

Totally agree. But I would say it's not just lack of experience. It seems a lot of the people now who have 2 years of experience are "Leads" or "Seniors" and they have a very dogmatic way of thinking based on the the exact trends and headaches you described.

We had a new lead join, whose first job was as a Lead(!) and he insisted on adding a pre-commit prettification hook. Now due to the specifics of our work this formatting creates all kinds of bugs and means the local code you work on doesnt match the work on the repo or server, which is terrible but I have no power to change things because they've been taught to believe that this is the only way.

View on HN

To be honest, I find that in the long term using an autoformatter greatly reduces what I call "diff spam". There's always that one guy using another IDE with his own auto-formatting tool that rewrites whole files. This is extremely annoying when reviewing merge requests and commit histories. Using an autoformatter solves this issue (and many other formatting related timesinks).

In addition, I think for most use cases having formatting-dependent code is kind of bad practice (not talking about indentation based code scoping à la python here). Although I do have the occasional struggle with ascii diagrams / comment formatting.

All-in-all I understand both sides, with a slight preference for using one. Manually formatted code in some cases can be much more beautiful than autoformatted counterparts. It's certainly not something I would impose; rather a team-decision after a short discussion.

While I am a fan of auto-formatting, I do use it sometimes and don't think it is necessarily dumb in all cases (and so agree with your premise of making it a team decision)... but, doing it as a pre-commit hook--as opposed to doing it when your file gets saved and merely verifying it was done pre-commit--is extremely dangerous as it leads to these issues like "code is tested locally... and then something different is committed that should be the same, but might not be" and is the real problem being described.

For example, I recently spent several hours fixing a pre-commit hook that would apply clang-format to any file changed and then `git add` the file.

Thus, the patch shown in `git commit --verbose` would be very misleading. It would show the original patch, not what was actually going to be committed. Due to the hook, any other changes that happened to be in the same file would be silently commited (even if they were never staged for commit).

(OMG I somehow edited this sentence so many times I dropped a "not": I am not a fan of auto-formatting ;P.)

It's really a trade-off. If the benefits outweight the possible negatives then I agree but in our case it's clearly not good, since the code being sent for review does not match the code on your local dev environment.

Worst case scenario you can format code on your local machine using your preferred formatting rules. I prefer to do this with the A and B documents since they will be using the exact same formatting (B comes from a different repo/team without our pre commit prettification).

But how did we get to the point when a full-source patch is even acceptable? Back in the day I’d come to this guy to ask why a single feature modified every place and question their understanding of what we use version control for. Why do we give a fuck about some funny IDEs which do not get it either?

That points us to the source of all the complexity we have now: it’s not newbies who broke things, it was tooling written by complete idiots.

Putting an auto formatter as a commit hook is just one of many small things that accumulate to make your develpment experience frustrating and annoying.

If you have a guy who always does a diff spam, just talk to him and ask him to separate his commits and minimze the diff spam. You don't need to ruin everyone's experience just to counter that.

Well, I think it is because so many people don't use version control in the intended way nowadays. I always review my changes before committing / pushing, but I have the impression many developers nowadays just stage, commit and push, as if VC is some sort of simple storage mechanism. I have talked to numerous people over the past few years to ask them to disable auto-formatting and/or to check their changes before pushing, but alas not always with the intended result. In the end I decided to push for using auto-formatting commit hooks in some projects (even though I dislike it on my own code).

An unfortunate observation: the majority of these projects were bloated, frontend javascript applications. In complicated backend applications I seem to encounter a higher SNR, and thus no necessity for commit-hooks.

Ultimately version control is about easily merging changes from people working on different machines.

Being able to "review" changes as clean set of patches is not essential.

The mindset of "we must review every change before it goes in" is not based in reality anyway.

Have you ever seen a system of code reviews produce useful benefits that would not be there without it?

You can always review already-committed code and discuss it within the team. It does not need to happen at commit time.

> pre-commit prettification hook

At my work we have this the "right" way in C# with a gerrit flow: run "dotnet-format --check" in the pre-push hook. So you can locally commit what you want, it must be checked before pushing, and it doesn't automatically change anything without asking you or letting you review it.

> due to the specifics of our work this formatting creates all kinds of bugs and means the local code you work on doesn't match the work on the repo or server, which is terrible but I have no power to change things because they've been taught to believe that this is the only way.

.. but this is the real problem.

You could suggest that this hook only be executed on a small subset of the repo to start with.

I have. I suggested limiting it to certain unproblematic areas. But, its a dogmatic all or nothing type of attitude unfortunately.

Reply on news.ycombinator.com