On Thu, Dec 13, 2012 at 01:58:52PM +0100, Oliver Kiddle wrote:
>> I would greatly appreciate it if someone would generate a summary of the
>> conclusions of this discussion with all the various "recommended" steps
>> that one would perform, so that it could be added as a sort of how-to in
>> Etc/zsh-development-guide.
>
> Ok, how does this look?
Looks good to me, see below for some minor comments.
> The zsh patch archive doesn't exist any more does it?
>
> Having prepared this with git, I realise that "git format-patch" doesn't
> really fit with the way we're used to working. It produces full e-mail
> headers with the expectation that the commit message forms the actual
> body of the e-mail. It also uses [PATCH] instead of PATCH: though that's
I've configured mutt's $editor to fix the [PATCH] subject line,
but maybe we could change the policy to allow [PATCH] in the
subject. Last time I checked git format-patch can't create
patches without the [].
> minor. As with this message, we often include a patch in a reply. Should
> we recommend something else?
Those mails (for example this mail) work fine with git am. The
only problems are the mail subject and the body (for the commit
message). The committer could either use git commit --amend or
git am -i to fix the commit message. It's not too much trouble
and just requires adding the X-Seq and fixing the commit message
in your $editor.
Sadly git-am doesn't provide any hooks which get the original
patch message as argument. Therefore we can't extract it
automatically when applying the patch (extending git-am to add
another hook shouldn't be too hard though). Alternatively we
could provide a little wrapper script which is used to apply Zsh
patches and extracts the X-Seq and adapts the subject (I could
write it if you think it's a good idea).
But we could use pre-applyhook to prevent accidental commits
without a X-Seq number in the commit message (checks can be
skipped with commit --no-verify).
We could also use git-am/git-format-patch's scissors feature (man
git-format-patch | less -p '^DISCUSSION'), but I think it just
creates unnecessary burden for the patch sender.
> diff --git a/Etc/zsh-development-guide b/Etc/zsh-development-guide
> index db78f94..cf7ff1c 100644
> --- a/Etc/zsh-development-guide
> +++ b/Etc/zsh-development-guide
> @@ -32,6 +32,7 @@ Patches
> -------
>
> * Send all patches to the mailing list rather than directly to me.
> + Use a Subject: line starting with "PATCH:".
Maybe we should allow [PATCH] as well.
> * Send only context diffs "diff -c oldfile newfile" or unified diffs
> "diff -u oldfile newfile". They are much easier to read and
> @@ -39,7 +40,7 @@ Patches
> intelligently. Please make sure the filenames in the diff header
> are relative to the top-level directory of the zsh distribution; for
> example, it should say "Src/init.c" rather than "init.c" or
> - "zsh/Src/init.c".
> + "zsh/Src/init.c". Alternatively, use "git format-patch".
>
> * Please put only one bug fix or feature enhancement in a single patch and
> only one patch per mail message. This helps me to multiplex the many
> @@ -52,9 +53,37 @@ Patches
> * Please test your patch and make sure it applies cleanly. It takes
> considerably more time to manually merge a patch into the baseline code.
>
> -* There is now a zsh patch archive. To have your patches appear in the
> - archive, send them to the mailing list with a Subject: line starting
> - with "PATCH:".
> +Git Workflow
> +------------
> +
> +* To allow changesets to be cross-referenced between the mailing list
> + archives and version control history, commit messages should start with
> + the mailing list sequence number. This number is generated by the list
> + server and inserted as an X-Seq: header field in the e-mail. To add
> + the number, you can use "git commit --amend" to change the commit.
Maybe we should add:
before pushing it to zsh's repository.
To make it clear that --amend is only used in the local
repository but not for published commits.
> [snip]
> +
> +* Patches can be prepared for the mailing list with "git format-patch
> + origin/master". To apply patches from a mailing list message, you can
> + use "git am".
I think we should add some information for committers how to
"fix" patches sent to the mailing list (e.g. git am -i or git
commit --amend, maybe combined with the custom wrapper which I
mentioned above).
Regards,
Simon
--
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
Attachment:
pgpghmUiAoxi_.pgp
Description: PGP signature