Hacker News Reader

Comment by hobofan | Hacker News Reader

I'm surprised they went with a all-at-once reformat. Even when doing it over a weekend this is bound to mess with a lot of open PRs at their scale.

I had to introduce a formatter in a few sizeable codebases in the past (few 100k to few million LOC), and I always did it incrementally via a script that reformatted all files that are not touched in any open PR. The initial run reformatted 95% of all files. Then I ran the script every day for ~two weeks and got up to 99.5% of all files and then manually each time one of the remaining ~dozen PRs that were WIP for longer were merged.

View on HN

both options have their pros and cons. if you utilize some form of ratcheting[1], you can sneak it in without your team knowing.. but all of your PRs for the foreseeable future will have a ton of reformatting screwing with your git blame. if you do it all at once, someone will have to sort out conflicts, but you can utilize `blame.ignoreRevsFile`[2] so that your history remains useful

[1] https://github.com/diffplug/spotless/tree/main/plugin-gradle...

[2] https://git-scm.com/docs/git-blame#Documentation/git-blame.t...

Yes, that is a good point. This is also why I personally would recommend to let a central person/team handle the reformatting rather than sneaking it into every PR (- see my sibling comment). That way you can be in charge of having a uniform style of commit messages to make the reformat commits easy to identify and create a well kept ignoreRevsFile. I think that provides the best of both worlds.

That’s a neat feature, thanks for sharing.

Unfortunately I find that code bases lacking auto formatting are often littered with non functional changes as developers temporarily instrument code, remove it, but leave whitespace changes behind.

In terms of tracking code changes, one really would have to rewrite the entire history with each commit reformatted.

Even if you spread out reformatting over multiple commits in different PRs, you can still make use of blame.ignoreRevsFile as long as your pull request workflow doesn't enforce squash merges even when somebody took extra care to produce a nice commit history.

> I'm surprised they went with a all-at-once reformat. Even when doing it over a weekend this is bound to mess with a lot of open PRs at their scale.

Rebasing PRs should be trivial, just rewrite all commits reformatting the files it touches, then rebase, `git checkout --theirs`, run formatter again, and `git rebase --continue`. It's methodical and scriptable, you don't need to manually resolve any conflicts.

You can always let the team know so that they can apply the formatter on their PR branch.

This is exactly the remedy to the PR issue. I've "lucked" into owning a Prettier formatting pass at two different places now, and did the same process at each - full pass on master, simple step-by-step process to follow to update any PR by running the format script.

In the smaller migrations I did I tried that, but some way or another a decent chunk of the people still managed to get stuck in merge/rebase conflicts. I would almost explicitly not recommend giving that advise to the teams.

My rough blueprint for introducing formatter or linter nowadys would be:

- Recorded knowledge share session around how to set up the tools for local use 1-2 weeks before the initial rollout, and outline how the process will take place

- On the day of the initial rollout send out a reminder + the recording again

- Do the initial PR

- Incrementally do the rest of the migration, and subscribe to the PRs that drag out the process

It's probably because the author can shit on others (let me guess, a senior principal something engineer).

Reply on news.ycombinator.com