Re: Changes to "Contribution Checklist" -- Format of the contribution.

[Florian Weimer <fweimer@redhat.com>]

* Carlos O'Donell:

> Proposed:
> https://sourceware.org/glibc/wiki/Contribution%20checklist%20v2
> - Shorter. All version specific information moved out to
>   "Legacy Contributions" page.
> - Streamlined. Steps in order of relevance and patch creation
>   process.
> - Focus on getting full patches via `git format-patch` from
>   developers, and thus making review easy.
> - Suggest use of 8< to split discussion from patch
>   (see git-mailinfo).
> - Fixed up any old information.

Thanks for doing this.  Here are my comments.

* Bugzilla Entry

I find this confusing.  I think we call those “bugs”, not “entries”.

| If your contribution fixes a user-visible issue it must have a
| bugzilla entry.

User-visible issue *in a released version*, I think.  It also makes
sense to file bugs for issues discovered during

: If your contribution fixes a user-visible defect present in a released
: glibc version (or has been discovered during a release freeze), it
: must reference a bug in Bugzilla.  Please also reference any bug that
: has been filed even if it is not strictly needed by the preceding rule
: (e.g., if the change concerns an enhancement, not a defect).

Please capitalize “Bugzilla” consistently.

* General Patch Requirements

I think we are mixing what should be in the posted email with what
should go into the actual commit.  E.g., should test reports be in the
commit message or not?  There are also general development instructions
(such as identifying related changes).

I think this would be improved by separating the three steps: patch
development, testing, commit formatting, and mailing list posting.

I don't think we should suggest that scripts/build-many-glibcs.py
improves a contribution.  Not everyone will be able to run it.

* Proper Formatted Unified diff of the Changes

Suggest addition:

: Make sure that your commit is on top of a commit which is already in
: the public Git tree, so that your collaborators can apply it using
: `git am -3`.

(Otherwise the command will likely fail.)

* Contribution Email Subject Line

This is not followed at all, so I think we can substantially consolidate
this section.

This section should be titled “Commit subject line”, which is what the
developer will actually produce before posting to the list, and come
earlier in the checklist (after development/testing).

Suggest instead:

: The first line of the commit message should contain the following:
: 
: * Subsystem (`argp`, `malloc`, etc., generally a directory name in the
:   source tree), kernel (`Linux` or `Hurd`), or architecture (e.g., `x86`
:   for all three variants).  This can be omitted for generic changes
:   that do not fit to a particular subsystem.
: * A colon (`:`), if the first field is present.
: * A brief summary of the change (~50 characters long).
: * The referenced bug(s) in Bugzilla (separated by a space from the
:   previous).
: 
: Here a some examples of commit subjects:
:
: * `nss: Add tst-nss-files-hosts-long test [BZ #21915]`
: * `powerpc: Fix VSCR position in ucontext (bug 24088)`
: * `Add fall-through comments.`

* Contribution Email Body

These conflicts with the `git format-patch` recommendation.  We should
describe two ways of posting `git am`-consumable patches:

* `git format-patch` output with manual edits in the ignored places.
* `git format-patch` output posted as an attachment.

I don't know if we can get `git am` to automatically detect which case
is which.

* Attribution

I still think we should ban `Signed-off-by` because it seems to imply
that the submission is *not* subject to the FSF copyright assignment.

We need to distinguish between authors and entities with copyright
ownership.  Only the latter need to be identified.

I also suggests this change:

: "Contributed by" statements *in source code comments* are no longer
: required or desired in glibc source files (though existing statements
: will remain).

This should probably go under the development section, though.

Thanks,
Florian