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 <tgamb...@baylibre.com> > > > --- > > > 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 :/ Cheers, Richard
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#205162): https://lists.openembedded.org/g/openembedded-core/message/205162 Mute This Topic: https://lists.openembedded.org/mt/108626678/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
