recommendation for patch maintenance
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi.
I would like to have advices about some possible workflows to use when maintaining a patch, that can evolve over the time (fixing bugs, and applying advices from reviewers).
In my case I have a single commit to maintain.
The workflow I use now is this:
1) create a topic branch, e.g. mp/complete-path
2) write code
3) commit
4) format-patch --output=mp/complete-patch master
5) review the patch
6) send-email
when I need to update the patch:
1) modify code
2) commit --amend
3) format-patch --subject-prefix="PATCH v<n>" \
--output=mp/complete-patch master
4) edit patch to add a list of what was changed
5) review the patch
6) send-email
This is far from ideal, since all my local changes are lost. Another problem is that when I found some trivial error in 5), I need to call format-patch again, loosing the "what's changed list".
A possible solution is to:
1) create a "public" topic branch, e.g. mp/complete-patch
2) create the associated "private" topic branch, e.g. mp/complete-patch/private
...
Changes are committed to the private branch.
When I need to update the patch:
1) update code
2) commit new changes; the commit message will contain the "what's changed" list to be used for the new version of the patch
3) checkout <public branch>
4) merge --squash <private branch>
Now I have my full history, and the "what's changed list" is saved in the private commits.
(not tested)
What is the workflow you usually use?
Thanks Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDUde4ACgkQscQJ24LbaUSqOwCfZON5f9mdAYkvACim802JGFhP 5W8An1Y7WXgsH/Q/p1/0jVMo1dJ3HwwO
=Xydn
-----END PGP SIGNATURE-----
Quoted 26 lines · Manlio Perillo
I would like to have advices about some possible workflows to use when maintaining a patch, that can evolve over the time (fixing bugs, and applying advices from reviewers).
In my case I have a single commit to maintain.
The workflow I use now is this:
1) create a topic branch, e.g. mp/complete-path
2) write code
3) commit
4) format-patch --output=mp/complete-patch master
5) review the patch
6) send-emailwhen I need to update the patch:
1) modify code
2) commit --amend
3) format-patch --subject-prefix="PATCH v<n>" \
--output=mp/complete-patch master
4) edit patch to add a list of what was changed
5) review the patch
6) send-email
Quoted 1 line
This is far from ideal, since all my local changes are lost.
Not offering any answer, but it is unclear to me what local changes you are losing here. Care to explain?
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 21/12/2012 18:01, Junio C Hamano ha scritto:
Quoted 17 lines
Quoted 4 lines · Manlio Perillo
I would like to have advices about some possible workflows to use when maintaining a patch, that can evolve over the time (fixing bugs, and applying advices from reviewers).
[...]
Quoted 10 lines
when I need to update the patch:
1) modify code
2) commit --amend
3) format-patch --subject-prefix="PATCH v<n>" \
--output=mp/complete-patch master
4) edit patch to add a list of what was changed
5) review the patch
6) send-email
Quoted 4 lines
Quoted 1 line
This is far from ideal, since all my local changes are lost.
Not offering any answer, but it is unclear to me what local changes you are losing here. Care to explain?
I lose the history of all the changes I have made to produce the final version of a patch.
Since for every new version of a patch I do a commit --amend, I can not see, as an example, the changes I have made between x and y versions of a patch.
Of course the commits are not really lost, but I have to search them using the reflog.
Thanks Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDUnIAACgkQscQJ24LbaUTf0QCfX9WtA+/GLzVWDJFPbLMCPucJ bKQAnj0HJuQs9SVCPV/TlDXcpGDqIqfD
=lhZ5
-----END PGP SIGNATURE-----
Quoted 9 lines · Manlio Perillo
I lose the history of all the changes I have made to produce the final version of a patch.
Since for every new version of a patch I do a commit --amend, I can not see, as an example, the changes I have made between x and y versions of a patch.
Of course the commits are not really lost, but I have to search them using the reflog.
Yeah, these days with "rebase -i" that does not leave individual steps as separate reflog entries, for an easy series, you can do things like:
$ git rebase -i ;# work on polishing the series
$ git show-branch -g ;# view where it diverged
$ git diff HEAD~4 @{1}~4Of course you can plan ahead (this is what I usually do when working on a series that is not "how about this" throw-away patch I send to this list all the time) and name the topic "topic-v1", fork the new round "topic-v2", "topic-v3", etc. and do things like
$ sh -c 'git diff topic-v2~$1 topic-v3~$1' - 4
(the point being that then you do ^P or whatever shell command history mechanism to recall that line, edit "4" to "3" and rerun the comparison for other corresponding ones).
On a related but different front, I've been thinking about improving the "format-patch --subject-prefix" mechanism to make it even easier to use. With the current interface, when you prepare your reroll, you would do:
$ git format-patch --cover-letter \
--subject-prefix='PATCH v4' -o ./+mp masterand end up overwriting the patches from the previous round (if their subject did not change). You will always overwrite the cover because its subject never changes and that is where the filename is taken from.
What if we added another option, say --reroll $n (or -v$n), to let you write:
$ git format-patch --cover-letter -v4 -o ./+mp master
that produces output files named like:
./+mp/v4-0000-cover-letter.txt
./+mp/v4-0001-git-completion.bash.add-support-for-pa.txtwith the subject '[PATCH v4]' prefix?
Then you can transplant the cover letter material from the cover letter from the older iteration to the new cover letter in your editor, and sending them out will become
$ git send-email ... ./+mp/v4-*.txt
Hmm?
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 21/12/2012 19:17, Junio C Hamano ha scritto:
Quoted 6 lines
[...]
Of course you can plan ahead (this is what I usually do when working on a series that is not "how about this" throw-away patch I send to this list all the time) and name the topic "topic-v1", fork the new round "topic-v2", "topic-v3", etc. and do things like
Quoted 6 lines
$ sh -c 'git diff topic-v2~$1 topic-v3~$1' - 4(the point being that then you do ^P or whatever shell command history mechanism to recall that line, edit "4" to "3" and rerun the comparison for other corresponding ones).
Thanks, this seems fine.
Maybe topic-v2 --> topic/v2 (it looks better to me).
Quoted 9 lines
On a related but different front, I've been thinking about improving the "format-patch --subject-prefix" mechanism to make it even easier to use.
[...]
What if we added another option, say --reroll $n (or -v$n), to let you write:
Quoted 9 lines
$ git format-patch --cover-letter -v4 -o ./+mp masterthat produces output files named like:
./+mp/v4-0000-cover-letter.txt ./+mp/v4-0001-git-completion.bash.add-support-for-pa.txtwith the subject '[PATCH v4]' prefix?
I think it is a good idea to reduce the things one have to do by hand. I, too, was thinking something similar, with a -v$n option.
And, from these few days I have started to follow the mailing list, it seems that '[PATCH v$n'] is the standard practice.
By the way, I would also like to be able to set the default value for the --output option in config file; something like:
[format] output = +mp/$(git symbolic-ref --short HEAD)
where the string will be processed by the shell:
sh -c 'printf "+mp/$(git symbolic-ref --short HEAD)'"
Note that printf is POSIX, and the standard suggests to use it instead of echo:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html
I have read the affected source code, and it should not be too hard. What do you think?
I should be able to hack the patch in the weekend, but I'm not sure it will be accepted.
Quoted 7 lines
Then you can transplant the cover letter material from the cover letter from the older iteration to the new cover letter in your editor, and sending them out will become
$ git send-email ... ./+mp/v4-*.txtHmm?
Seems good.
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDU1REACgkQscQJ24LbaUT5dgCgismeh+R3B26otuBIXRf/VUiq +5gAoIR56wVfs8Vw8AAWtim4aor97MXN
=DfeG
-----END PGP SIGNATURE-----
Quoted 10 lines · Manlio Perillo
By the way, I would also like to be able to set the default value for the --output option in config file; something like:
[format] output = +mp/$(git symbolic-ref --short HEAD)where the string will be processed by the shell:
sh -c 'printf "+mp/$(git symbolic-ref --short HEAD)'"
My knee-jerk reaction is that, while it might make sense to give an option to allow users to use a dedicated directory per branch, I am not interested in that form; especially the part that lets the users too long a rope to hang themselves with by allowing an arbitrary shell expansions goes against my taste.
But you *can* prove me wrong; read on.
Quoted 2 lines
I should be able to hack the patch in the weekend, but I'm not sure it will be accepted.
You got it backwards, just like many other new people who come and go on this list. If something is very useful, you'd work on it even if the result may not appear immediately in the upstream, and you'd keep maintaining a local fork so that you can keep benefiting from it, once you have written such a useful feature (whatever it is). If even the original "itch-holder" does not think
that the benefit from his change outweighs the cost of such an effort, why should *I* carry it in my tree?
The goal of a contributor who comes up with an idea, gets "It doesn't sound like a good idea" during the review but disagrees and believes in his idea ought to be to prove the reviewer(s) wrong by polishing the idea and the implementation so much that the upstream comes begging for your change ;-)
The core part of the change to add --reroll $n option should be straight-forward and will involve a few functions in builtin/log.c, but the existing code is so badly
structured that it probably needs a couple of "preparatory steps" patch to clean up the API between internal functions on that codepath, before the real change can be made, I think.