From: Carlos O'Donell <carlos@redhat.com>
To: Florian Weimer <fweimer@redhat.com>
Cc: "Joseph S. Myers" <joseph@codesourcery.com>,
Andreas Schwab <schwab@suse.de>,
Adhemerval Zanella <adhemerval.zanella@linaro.org>,
Szabolcs Nagy <szabolcs.nagy@arm.com>,
GNU C Library <libc-alpha@sourceware.org>
Subject: Re: Changes to "Contribution Checklist" -- Format of the contribution.
Date: Wed, 13 Mar 2019 14:22:13 -0400 [thread overview]
Message-ID: <49e993ee-0954-a38a-e0e6-de2128c4bf71@redhat.com> (raw)
In-Reply-To: <87ftspjkao.fsf@oldenburg2.str.redhat.com>
On 2/15/19 7:41 AM, Florian Weimer wrote:
> * 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).
Fixed. Thank you for the text.
> Please capitalize “Bugzilla” consistently.
Fixed.
> * 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.
I like your suggestion to further break this into the steps that a normal
developer would follow.
> * 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.)
Added.
> * Contribution Email Subject Line
>
> This is not followed at all, so I think we can substantially consolidate
> this section.
Some of the text is OK, others have some variance.
> 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).
Agreed, I called it "First line" under "Commit formatting"
> 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)`
I added an example for multiple bugs: (bug xxxx, bug yyyy).
> : * `Add fall-through comments.`
I added this.
> * Contribution Email Body
>
> These conflicts with the `git format-patch` recommendation. We should
> describe two ways of posting `git am`-consumable patches:
Yes, I agree, I fixed this.
> * `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.
OK, I've done this.
> * 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.
I accept that this could be a problem.
I have removed it.
> We need to distinguish between authors and entities with copyright
> ownership. Only the latter need to be identified.
Fixed.
> 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.
Moved.
Please review the new docs.
--
Cheers,
Carlos.
prev parent reply other threads:[~2019-03-13 18:22 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-13 21:33 Changes to "Contribution Checklist" -- Format of the contribution Carlos O'Donell
2019-02-13 22:19 ` Paul Eggert
2019-02-13 23:00 ` Joseph Myers
2019-02-14 0:11 ` Carlos O'Donell
2019-02-14 1:38 ` Joseph Myers
2019-02-14 9:58 ` Florian Weimer
2019-02-14 17:32 ` Joseph Myers
2019-02-15 11:54 ` Florian Weimer
2019-02-14 20:17 ` Carlos O'Donell
2019-02-15 12:41 ` Florian Weimer
2019-02-15 13:53 ` Joseph Myers
2019-03-13 15:47 ` Florian Weimer
2019-03-13 21:35 ` Joseph Myers
2019-03-13 21:40 ` Carlos O'Donell
2019-03-13 22:02 ` Florian Weimer
2019-03-24 17:18 ` Gabriel F. T. Gomes
2019-03-13 21:54 ` Florian Weimer
2019-03-13 18:22 ` Carlos O'Donell [this message]
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
List information: https://www.gnu.org/software/libc/involved.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49e993ee-0954-a38a-e0e6-de2128c4bf71@redhat.com \
--to=carlos@redhat.com \
--cc=adhemerval.zanella@linaro.org \
--cc=fweimer@redhat.com \
--cc=joseph@codesourcery.com \
--cc=libc-alpha@sourceware.org \
--cc=schwab@suse.de \
--cc=szabolcs.nagy@arm.com \
/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).