undefined | Better HN

0 pointsKoffiepoeder4y ago0 comments

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.

0 comments

8 comments · 4 top-level

saurik4y ago· 2 in thread

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.

slavik814y ago

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).

saurik4y ago

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

hsn9154y ago· 2 in thread

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.

KoffiepoederOP4y ago

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.

hsn9154y ago

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.

wruza4y ago

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.

mouzogu4y ago

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).

j / k navigate · click thread line to collapse