Differences between revisions 35 and 36
Revision 35 as of 2015-05-18 09:09:53
Size: 14202
Editor: GaryBenson
Revision 36 as of 2015-06-25 13:33:16
Size: 14282
Editor: DougEvans
Deletions are marked like this. Additions are marked like this.
Line 106: Line 106:
 * Don't add new tests that are known to FAIL. Mark them as KFAIL or XFAIL.

Contribution Checklist

A high-quality contribution includes the following things:

1. Contribution Email Subject Line

A high-quality email subject line for a contribution contains the following tags:

For a Single patch contribution:

  • [PATCH] <Subject>

For Multi-part patch set contributions where [PATCH 0/2] is used to described the forth coming patches:

  • [PATCH 0/2] <Optional unique prefix e.g. "String cleanup:"><Subject related to the patch set e.g. Cleanup all broken string routines.>

  • [PATCH 1/2] <Optional unique prefix e.g. "String cleanup:"><Distinct subject line for this patch e.g. Cleanup strstr.>

  • [PATCH 2/2] <Optional unique prefix e.g. "String cleanup:"><Another distinct subject line for this patch e.g. Cleanup strncmp.>

For a request for comments that doesn't have to be a patch at all:

  • [RFC] <Subject>

For a request for comments on a patch or patch set:

  • [RFC][PATCH] <Subject>

For a patch that fixes a bug filed in bugzilla:

  • [PATCH][PR <component/number>] <Subject>

For a patch that is the Nth version of an earlier patch:

  • [PATCH vN] <Subject>

For a patch that is already committed:

  • [COMMITTED PATCH] <Subject>

For a patch that falls under the The Obvious Rule as described in gdb/MAINTAINERS:

  • [OB PATCH] <Subject>

Most reviewers get a lot of email. To help them, please make <Subject> as descriptive as possible, with the most important words early on:

  • Fix PR 14963 is a bad subject: everybody on the list has to open this email to see what it relates to.

  • Fix segfault is a little better: we can see this patch fixes an important issue, but in which component?

  • Fix segfault caused by infinite loop in demangler is better, but the component is at the end and may be truncated in people's email clients.

  • Fix segfault demangling C++11 rvalue references is pretty great.

2. Detailed Explanation of the Patch

2.1. General requirements

  • Describe the rationale for the patch. E.g., don't assume the change will be obvious to others just because it's a small patch. It may be that it only looks obvious to you, because you've spent the effort investigating the issue, and you have all the context in your head. Your job is to make it easy for the reviewer to give you an OK.
  • If you experimented other seemingly valid or seemingly better approaches for your fix/feature, and they didn't work out, mention it in the patch submission, along with the reason for why it didn't work out.
  • Explain the steps required to reproduce the problem.
    • Extra points are awarded for a test-case which demonstrates the problem and can be used in the test-suite to demonstrate success of a fix.
  • Explain the observed versus expected behaviour. If you're changing the output of some command, include a paste of the relevant parts of gdb session, before and after the change.
  • Explain how you tested the patch. If you ran the testsuite, say so, and say on which targets. If you didn't run the testsuite, you should have. If you still haven't run the testsuite, say so, and explain why.

2.2. ChangeLog

All patches for consideration must have ChangeLog entry updates (see below) of they will be rejected during mailing list moderation.

  • Run find . -name ChangeLog to help identify the correct ChangeLog file. E.g.,

    • ./ChangeLog
  • This should not be part of the change diff; paste it in clear text in the body of the email.
  • When posting a new version of a patch, either as a new thread, or in reply to an existing thread, always repost the ChangeLog entry as well, even if it hasn't changed.

  • If the code in question is a Request For Comment (RFC) please indicate as such in the email subject. Explicit RFC emails aren't constrained by the ChangeLog rules but they won't be considered for inclusion until they comply.

3. Documentation

If your patch adds a new command then that command should be documented in the GDB manual, and mentioned in the NEWS file. All commands must be documented, including maintenance commands (e.g., "set debug" or "maint" commands).

New target ports must be mentioned in the NEWS file.

New host configurations must be mentioned in the NEWS file.

New remote protocol packets must be documented in the GDB manual, and mentioned in the NEWS file.

Standard target features for xml target descriptions (e.g., org.gnu.gdb.arm.core, org.gnu.gdb.i386.sse) must be documented in the GDB manual.

New MI commands, attributes, etc., must be documented in the GDB manual, and mentioned in the NEWS file.

New Python scripting features must be documented in the GDB manual, and mentioned in the NEWS file.

4. Testing

  • Make sure there are no regressions in the test suite.

  • Explain for which targets the test suite was run.
  • Adding a test-case to the test suite may increase the likelihood of acceptance of your patch. In many cases it will be necessary.
  • Don't add new tests that are known to FAIL. Mark them as KFAIL or XFAIL.
  • See here for a cookbook on writing GDB tests.

  • Explain your current FSF copyright assignment status.
  • In the event that you have not previously completed an FSF copyright assignment, a GDB maintainer will direct you to a FSF copyright assignment request form that's best suited to your contribution under the guidelines on this gnu.org webpage.

  • As from August, 2013, the FSF began accepting GPG signed assignments from U.S. residents. More information at this link.

  • All source files must have a copyright header. Although the FSF only requires that for legally significant files (IOW, files with only a couple lines wouldn't require a header), GDB maintainers prefer not having to worry about later having to add the header when the file grows.

In addition, we track which files don't have copyright headers, and it improves the S/N ratio of the output of updating copyright years for each new year if every file has a copyright header. Thus a copyright header is required even for simple 3 line testcases. This GNU Copyright-Notices webpage indicates the proper format for the copyright notice and how to write the dates:

  • A copyright notice takes the following form:

     Copyright (C) year copyright-holder
  • For GDB, spanning of consecutive years IS allowed. Therefore, when adding a second (or more) consecutive year to the copyright statement, you may write a year range, e.g.,

     Copyright (C) year1-year2 copyright-holder

The simplest is to just copy the header from some other file.

If you've based your new file on some other file, as e.g., it is common to do when writing new tests, then the copyright years of the original file must be retained in the new file.

7. Properly Formatted GNU ChangeLog

  • Your ChangeLog must be in the GNU ChangeLog format. The ChangeLog format is described here and here.

  • Files that were regenerated should be identified and the change should say "Regenerate."
  • The GCC folks have a tool called mklog to help generate ChangeLogs. This script creates a ChangeLog template for a diff file. Run "mklog diff-file" and the template will be added to the top of the file.

  • Every change line in a ChangeLog should start with a capitalized word and end with a period.

  • The ChangeLog should indicate what was changed in the file. For instance, if you change the size of a variable you need to indicate as much in the ChangeLog.

  • ChangeLog standard tab-width is 8.

  • ChangeLog standard max-line-length is 74.

  • An example for a trivial change follows: Note: words in arrow brackets shouldn't appear in the ChangeLog, they indicate whitespace.

YYYY-MM-DD<2 spaces>John Doe<2 spaces><johndoe@some.email.address>
<tab--->PR <component>/<number><use this if there is a bugzilla PR associated with the patch>
<tab--->* breakpoint.c (handle_some_event): Remove reference to foo.  Call<line wrap at or before column 74>
<tab--->* configure.ac (build_warnings): Do not use -Wformat-nonliteral
<tab--->with -Wno-format.
<tab--->* configure: Regenerate.

2013-12-12  John Doe  <johndoe@some.email.address>

        PR gdb/9999
        * breakpoint.c (handle_some_event): Remove reference to foo.  Call
        * configure.ac (build_warnings): Do not use -Wformat-nonliteral
        with -Wno-format.
        * configure: Regenerate.
        * NEWS: Mention awesome feature.

It's okay (and not uncommon) to include a one sentence explanation to the patch as a whole, in addition or in place of "PR comp/xyz". E.g.,

YYYY-MM-DD<2 spaces>John Doe<2 spaces><johndoe@some.email.address>
<tab--->Short explanation.
<tab--->* (install_breakpoint): Remove reference to bar.

2013-11-11  John Doe  <johndoe@some.email.address>

        Code cleanup.
        * breakpoint.c (install_breakpoint): Remove reference to bar.

8. Properly Formatted Changes

Please see Coding Standards for a full description of coding guidelines.

See also JoelsCodingStyleCheatSheet for very common nits.

If you have any questions regarding these criteria please email gdb-patches@sourceware.org .

9. More info

The file <gdb-src>/gdb/CONTRIBUTE contains more instructions on what is expected from patches submitted to GDB.

10. Submitting patches

  • Make sure your mailer doesn't turn TABS into SPACES, i.e. turn on preformat.
  • Don't send patches with "format=flowed". This messes up line breaks and makes clients display tabs wrong.
    • Thunderbird users, see here.

  • See this excellent guide, that touches on the issues above, and more, including instructions for several mailers (Thunderbird, Alpine, KMail, Gmail, etc.).

11. Patch status

See GDB's Patchwork instance.

And with s/glibc/GDB/g in mind, read Patch Review Workflow.

12. Ping and keep pinging

If your patch is not reviewed it may just mean that the reviewers are busy. Please ping and keep pinging the patch on a weekly basis.

13. Properly formatted commit messages

When the time comes to push your commits, please ensure the commit messages are formatted as follows:

<single line summarizing the change>
<paragraph or paragraphs describing of the change>
<all changelog entries relating to this change>

Remember to include the PR number (in the ChangeLog entries) if you commit is to fix a bug reported in the bugzilla.

For example:

Make all source files include defs.h or server.h first

This commit makes all source files under gdb/ that include headers
from gdb/ include either defs.h or server.h before any other code.
This ensures that definitions and macros from the two config.h files
are always in place for our code.  An exception has been made for
gdb/gdbserver/gdbreplay.c which seems to be a special case.


        * btrace.c: Include defs.h.
        * common/ptid.c: Include defs.h or server.h as appropriate.
        * nat/mips-linux-watch.c: Likewise.


        * hostio-errno.c: Move server.h to top of includes list.
        * inferiors.c: Likewise.
        * linux-x86-low.c: Likewise.
        * notif.c: Include server.h.

The discussions leading up to this format are in this thread and this thread.

14. Fixing commit dates

If you have used git rebase to create your commits it is likely the commit dates are wrong. Before pushing please run this command:

git rebase --ignore-date origin/master

To set the timestamps of all the commits you are about to push to the current time.

None: ContributionChecklist (last edited 2015-06-25 13:33:16 by DougEvans)

All content (C) 2008 Free Software Foundation. For terms of use, redistribution, and modification, please see the WikiLicense page.