On 2024-10-01 08:56, Richard Purdie wrote:
On Tue, 2024-10-01 at 08:47 -0400, Trevor Gamblin wrote:
On 2024-10-01 07:45, Richard Purdie wrote:
On Tue, 2024-09-24 at 07:55 -0400, Trevor Gamblin via lists.openembedded.org
wrote:
Black makes things easier to read.
Signed-off-by: Trevor Gamblin <[email protected]>
---
meta/lib/patchtest/mbox.py | 44 +++---
meta/lib/patchtest/patchtest_parser.py | 106 ++++++++-----
meta/lib/patchtest/patchtest_patterns.py | 84 ++++++----
meta/lib/patchtest/repo.py | 32 ++--
meta/lib/patchtest/selftest/selftest | 92 ++++++++---
meta/lib/patchtest/tests/base.py | 86 ++++++-----
meta/lib/patchtest/tests/test_mbox.py | 144 +++++++++++++-----
meta/lib/patchtest/tests/test_metadata.py | 94 ++++++++----
meta/lib/patchtest/tests/test_patch.py | 39 +++--
.../lib/patchtest/tests/test_python_pylint.py | 45 ++++--
scripts/patchtest | 113 +++++++++-----
scripts/patchtest-get-branch | 54 ++++---
scripts/patchtest-send-results | 66 +++++---
13 files changed, 660 insertions(+), 339 deletions(-)
I'm a bit nervous about this change for a few reasons. Firstly, it sets
a precedent where others will want to do this on other areas of the
code and I'm not sure we really want to do that.
black is quite opinionated and I don't agree with everything it is
changing. Some of the code does look more readable before it made
changes compared to the end result, particularly around how it is
choosing to line wrap the code. Some of the changes are also just
rearranging things for little gain such as changing quoting characters.
I'm therefore in two minds of this. I do want to empower people to
maintain code but I don't want to end up forced into taking style
changes I really don't agree with :/.
I agree that it may lead to a rush of reformatting efforts that aren't
desired. We certainly don't have to take this patch now if you don't
want to, although the more minor changes (e.g. the quotations) were
actually a bigger part of the draw for me when I ran it.
On the other hand, some experience with using checkpatch.pl in the
kernel has me wondering if we should be considering standardizing our
formats for Python scripts and modules (over time) anyway. I have no
strong preference on which formatter would be best (to be honest, I
haven't even tried the others like YAPF or Ruff). Black, however, seems
to be the one recommended most, and though it's opinionated it helps
keep things consistent (the example above where the string quoting was
sometimes single and sometimes double drove me nuts every time I looked
at it).
Is this a reasonable goal for some distant release? I've CC'd Ross and
Tim since they're also doing some Python work. I might bring it up at
the weekly today.
Seemingly simple formatting changes have been some of the more
contentious issues in the past. The churn such patches introduce make
history difficult to read and make LTS maintanenace hard as patches
become hard to backport. I've "lived" through the discussion multiple
times in one format or another and I'm still not convinced that
adopting hard "rules" around format X or Y is a great idea.
We have been adopting a rough standardisation of things slowly over
time as code gets updated but it doesn't extend to the quotes or line
wrapping as there were other bigger problems. I'm not against doing
that as we update code but I'm not sure I'm convinced about block
formatting changes like this patch.
If there was a specific issue like the quoting, I'd in many ways prefer
to fix that in a specific patch rather than have multiple formatting
changes all mixed together. Our metadata quoting really doesn't help
things there :/. Different people are triggered by different issues, I
know personally quoting bothers much much less that function parameter
white spacing.
I worry this is a topic where there is no right answer too :/
Let's just drop it then, since Alex has raised an objection too. I'll
submit another one to fix the quoting at some point soon.
- Trevor
Cheers,
Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#205164):
https://lists.openembedded.org/g/openembedded-core/message/205164
Mute This Topic: https://lists.openembedded.org/mt/108626678/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
