Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
To: Andrew Burgess <andrew.burgess@embecosm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: Proposal: format GDB Python files with black
Date: Mon, 26 Apr 2021 13:25:37 -0400	[thread overview]
Message-ID: <ffd72b6e-ca44-cc0a-265a-cd94846e85da@polymtl.ca> (raw)
In-Reply-To: <20210426162149.GU2610@embecosm.com>

On 2021-04-26 12:21 p.m., Andrew Burgess wrote:
> * Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-04-26 11:55:21 -0400]:
> 
>> Hi all,
>>
>> A few people I talked to about this and I have good experience with the
>> tool black to auto-format Python code.  It is simple to use, fast and
>> reliable (if it detects that it changed the meaning of the program by
>> mistake, internal-errors out).  I don't think I need to spell out the
>> advantage of using such a tool, but just in case: it removes all the
>> overhead of thinking about formatting when writing or reviewing code.
>>
>> My suggestion would be to format our code with black all at once.  The
>> typical counter-argument to this is "it will be harder to use
>> git-blame".  I don't think this is true, because you need to be able to
>> skip over commits anyway, and it's trivial to skip over a commit when
>> git-blaming using an interactive tool.  But it's also possible to tell
>> git to ignore certain commits when git-blaming [2], so we can do that.
>>
>> I think the output of black is quite readable.  When it does weird
>> stuff, it's generally because the lines / expressions are two long
>> anyway, and deserve to be split in multiple lines / expressions.  Here's
>> a branch that shows how it would look like:
> 
> In general, the idea of auto-formatting the code sounds great, but I
> would like to understand more about how the process would work.
> 
> Right now, if I make a change I edit a file, git add, git commit, and
> finally, git push.  At what point does the auto-formatting take place?
> Do I need to manually run the tool after editing the file?  Is it
> done, well, automatically at some point?  And if it is done
> automatically, then at what point in the process does this happen, and
> how does this relate to code review?

Those are good questions.

At the simplest level, the individual making changes is responsible for
the tool at any time they want, as long as it's before sending the patch
and / or pushing to master.  So it would be pretty much like today,
where we are responsible for formatting the source code correctly,
except that it would now be tool-assisted instead of done mentally.

How to run the tool is left to each individual:

1. By hand on the command line
2. From the editor [1]
3. Using a git-commit hook

If we want to be a bit more sophisticated, we could check-in a git
commit hook that runs black (if available) to check your formatting as
you are committing, if your commit includes changes to Python files.
That doesn't prevents formatting errors from slipping in the repo, but
it's already a good start (and formatting errors slipping in the repo
are not the end of the world).

We don't really have CI right now (especially pre-merge testing), but if
we had, checking the formatting of Python files could be a validation
step.

We do however, have scripts that run server side on push, so it would be
possible to run black server-side to reject commits that would introduce
formatting errors.

Simon

[1] https://black.readthedocs.io/en/stable/editor_integration.html

  reply	other threads:[~2021-04-26 17:25 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-26 15:55 Simon Marchi via Gdb-patches
2021-04-26 16:21 ` Andrew Burgess
2021-04-26 17:25   ` Simon Marchi via Gdb-patches [this message]
2021-04-26 17:50     ` Andrew Burgess
2021-04-26 18:08       ` Simon Marchi via Gdb-patches
2021-04-27  7:54         ` Andrew Burgess
2021-04-27 13:21           ` Simon Marchi via Gdb-patches
2021-04-26 17:42   ` Tom Tromey
2021-04-26 17:34 ` Paul Koning via Gdb-patches
2021-04-26 17:44   ` Simon Marchi via Gdb-patches
2021-04-26 17:48     ` Simon Marchi via Gdb-patches
2021-04-26 17:39 ` Tom Tromey
2021-04-30 16:26   ` Joel Brobecker
2021-04-26 22:40 ` Lancelot SIX via Gdb-patches
2021-04-30 17:04   ` Tom Tromey
2021-04-30 17:14     ` Simon Marchi via Gdb-patches
2021-05-01  6:42       ` Joel Brobecker
2021-04-30 17:21 ` Luis Machado via Gdb-patches
2021-05-08 16:00 ` Tom Tromey
2021-05-11  2:55   ` Simon Marchi via Gdb-patches
2021-05-11  2:57     ` Using clang-format Simon Marchi via Gdb-patches
2021-05-11 13:31       ` Marco Barisione via Gdb-patches
2021-05-11 13:44         ` Simon Marchi via Gdb-patches
2021-05-11 20:40       ` Tom Tromey
2021-05-13 17:13         ` Simon Marchi via Gdb-patches
2021-05-11 11:38     ` Proposal: format GDB Python files with black Luis Machado via Gdb-patches
2021-05-11 13:49       ` Simon Marchi via Gdb-patches
2021-05-11 14:23         ` Luis Machado via Gdb-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ffd72b6e-ca44-cc0a-265a-cd94846e85da@polymtl.ca \
    --to=gdb-patches@sourceware.org \
    --cc=andrew.burgess@embecosm.com \
    --cc=simon.marchi@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox