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
___
Mailman-Developers mailing list -- mailman-developers@python.org
To unsubscribe send an email to mailman-developers-le...@python.org
https://mail.python.org/mailman3/lists/mailman-developers.python.org/
Mailman FAQ: https://wiki.list.org/x/AgA3
Security Policy: https://wiki.list.org/x/QIA9
