On 10/27/22 22:10, Stephen J. Turnbull wrote:
Abhilash Raj writes:
I am trying out some of the code formatting tools on Mailman repos to help reduce some efforts in development. The tools I am trying out is blue[1]
Please don't hurry to merge this to main/master. Remember that Python has been rejecting proposals to mass-PEP8 the stdlib for about 2 decades now.
So far, I did get it merged in Postorius, but that's the only one so far. Obviously git gives us powers to undo that, so we should still discuss this.
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?
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
On the other hand, I don't find any Mailman code hard or annoying to read because of stylistic differences.
That's true, the current state of code base isn't the motivation really for me personally :-)
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. My editor sometimes helps me with annoying red and yellow underlines, but I don't still see a need to have to resolve any formatting only error manually during development process.
From a contributor's perspective also, even though it is not a massive help, it reduces one less CI check (qa -- partially) you don't need to manually fix for.
I'll take a look at the PRs but it will probably be a while (weeks) before I have a good sense for just how much disruption is involved.
and isort[2]
I'm cautiously in favor of this. Standardizing import format is a big enough improvement when reading code to justify the relatively small amount of churn.
I am also adding a new tox command
tox -e formatthat should run the appropriate commands locally for users to run.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.
I am probably missing something here.
I guess people with long-lasting feature branches may find it convenient in rebasing their branches (but I think that's going to be a massive PITA regardless[1]). I think the documentation should be *very* opinionated about getting input from core devs before blue'ing a file.
Footnotes: [1] Does anybody have experience in rebasing branches across a massive reformatting by upstream? Are there best practices here?
Yeah, rebasing is going to be an issue, but I agree with you that it is an issue regardless.
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.
-- thanks, Abhilash Raj (maxking)