5 ms·

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 th

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.

sombremesa 5y ago
You could suggest that this hook only be executed on a small subset of the repo to start with.
- 5350-uiop-1130 5y ago
  I have. I suggested limiting it to certain unproblematic areas. But, its a dogmatic all or nothing type of attitude unfortunately.
Koffiepoeder 5y ago
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.
- hsn915 5y ago
  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.
  - Koffiepoeder 5y 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.
    - hsn915 5y 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.
- saurik 5y ago
  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.
pjc50 5y ago
> 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.

sombremesa 5y ago

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

5350-uiop-1130 5y ago
I have. I suggested limiting it to certain unproblematic areas. But, its a dogmatic all or nothing type of attitude unfortunately.

Koffiepoeder 5y ago

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.

hsn915 5y ago
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.
- Koffiepoeder 5y 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.
  - hsn915 5y 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.
saurik 5y ago
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.

pjc50 5y ago

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