public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: binutils@sourceware.org, gdb-patches@sourceware.org
Subject: small request regarding commits in binutils-gdb.git
Date: Wed, 15 Jan 2014 12:12:00 -0000	[thread overview]
Message-ID: <20140115121251.GM4639@adacore.com> (raw)

Hello,

Please forgive this message for all (most) of you who already know
this, but I happen to notice a few commits here and there, where
the revision log contains a copy/paste of the ChangeLog entry.

This was a pretty typical and accepted practice when were using CVS.
But, the convention with git tools is that the first line should
be the subject of the commit, and this subject should be followed
by an empty line before the rest of the revision log starts (if
there is text afterwards).

Below are examples of commit revision logs which follow that convention:

> daily update

or

> PR symtab/16426
>
>     * dwarf2read.c (dwarf2_get_dwz_file): Call gdb_bfd_record_inclusion.
>     (try_open_dwop_file): Ditto.

Here is one, however, that does not follow the convention:

>       PR gas/16434
>       * config/tc-z80.c (wrong_match): Provide format string to
>       as_warn.

In the example above, making the "PR gas/16434" by stripping
the leading spaces, and adding an empty line after it would
be sufficient.

However, I'd like to take this opportunity to suggest a practice
that some of us have started adopting on the GDB project, which
is to make the revision log something close to the email we'd send
to submit patch for including in binutils-gdb.git.

It would look like this:

> psymtab cleanup patch 3/3
>
> This last patch removes "partial" from the names of
> expand_partial_symbol_names and map_partial_symbol_filenames.
> It also renames expand_partial_symbol_names to match the
> struct quick_symbol_functions "method" that it wraps:
> expand_symtabs_matching.
>
> This patch also adds two parameters to expand_symtabs_matching
> so that it can fully wrap the underlying quick_symbol_functions method.
> This makes it usable in more places.
> I thought of having a cover function that still had the same
> signature as the old expand_partial_symbol_names function,
> but I couldn't think of a good name, and it wasn't clear it was
> worth it anyway.
>
> gdb/ChangeLog:
>
>     * symfile.h (expand_symtabs_matching): Renamed from
>     expand_partial_symbol_names.  Update prototype.
>     (map_symbol_filenames): Renamed from map_partial_symbol_filenames.
>     * symfile.c (expand_symtabs_matching): Renamed from

The upside is that it makes it easier for you to submit your patch,
even if it submitted a long time after you actually wrote the patch.
Just use "git send-email", and voila.

For the rest of the community, using a meaningful subject, one that
actually gives a hint at what it does, is something that I have
found to be really helpful. For instance, "Provide format string
to as_warn" can be more helpful when scanning through a list
of commits than "PR gas/16434".

I am also considering the idea of writing a small hook that
would reject new commits introducing commits where the "empty line
after subject" rule is not followed. Would that be an acceptable
restriction?

-- 
Joel

             reply	other threads:[~2014-01-15 12:12 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-15 12:12 Joel Brobecker [this message]
2014-01-15 15:28 ` Mike Frysinger
2014-01-15 15:51 ` Tom Tromey
2014-01-15 16:04 ` Eli Zaretskii
2014-01-15 16:25   ` Joel Brobecker
2014-01-15 16:57     ` Eli Zaretskii
2014-01-15 17:12       ` Mike Frysinger
2014-01-15 19:48         ` Fred Cooke
2014-01-15 20:02           ` Mike Frysinger
2014-01-15 20:38             ` Eli Zaretskii
2014-01-15 20:57               ` Mike Frysinger
2014-01-15 21:09                 ` Eli Zaretskii
2014-01-16  2:11                   ` Alan Modra
2014-01-16  2:17                   ` Joel Brobecker
2014-01-16  1:41             ` Fred Cooke
2014-01-16  2:41               ` Mike Frysinger
2014-01-15 16:50   ` Pedro Alves
2014-01-15 17:03     ` Tom Tromey
2014-01-15 18:07   ` Gary Benson
2014-01-16 11:41   ` Jose E. Marchesi
2014-01-15 19:31 ` Cary Coutant
2014-01-15 19:45   ` Tom Tromey

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=20140115121251.GM4639@adacore.com \
    --to=brobecker@adacore.com \
    --cc=binutils@sourceware.org \
    --cc=gdb-patches@sourceware.org \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).