Abhilash Raj writes:
I (speaking only for myself) have no objection going forward for new files and major refactorings. But if you do this all at once, I fear that most interesting diffs for the next year or so will be full of reformatting cruft,
I am not sure I understand this part. Do you mean to say we'd be getting too may PRs with "formatting-only" changes?
No, I mean the diffs we do to localize errors. The day after you merge the reformatting PR, anybody who wants to identify a regression since Postorius 1.3.7 is going to get as many lines of " -> ' as they do of code changes. Half the time what appears to be a regression from future 1.3.8 to HEAD is going to turn out to be older than the reformat merge, same problem just less frequent. It gets less frequent over time, but it will take quite a while before the reformat merge consistently contributes less than 10% of changes, I expect.
Currently, my thought is to do a one-time format for the entire codebase and then add "blue --check" as a part of the "qa" CI gate. Then, for each subsequent MR that follows, people would need to run "tox -e format" before they commit and create MR. The hope there is also
Yes, this is a great idea for new files, or files you're refactoring to death anyway. But it has a potentially large downside of turning diffs that should be one-line changes into rafts of fixquote changes.
My motivation here is to not have to push "oops, make flake8 gods happy" commits on top of my PRs and not have to worry about it when writing code.
While it isn't too huge of an issue, it is still manual effort that I feel can be automated to reduce some work and save time.
Sure, but doing it globally also makes some work. The question is does it make more work (and annoyance) for everybody who does diffs than is experienced by the developer who needs to make a flake8 commit? I do a lot more diffs on Mailman than I do commits.
What is the purpose of this? Anybody who runs it on a code base before the blue'd code gets merged is going to generate hard to read crufty PRs, no?
How so? If the existing code base is already auto-formatted, then the PRs would be just regular diffs, formatted with the same tool.
Maybe tox -e format isn't a problem because you would add that tox target in the same commit (or later) you do the reformat, but if this is going to be policy, some people will aggressively reformat their code to that standard before the package is reformatted. Every once in a while you still see MRs in Python where somebody PEP8s a whole stdlib module where the contribution is a 2-line doc improvement. Those get rejected of course. We should do that too.
I've tried to solve at least the "git-blame" issue by adding the rev of the commit with the "mass-refactor" into a file in the main repo that you can feed to "git blame --ignore-rev-file .git-blame-ignore-revs".
For editors, there is a way to set git config for the blame-ignore-file so it works across everything that uses git.
That's very helpful for some operations. How to make it discoverable is something of a headache though -- I didn't know there was a blame-ignore-file!
Steve