* recommendation for patch maintenance
@ 2012-12-21 14:45 Manlio Perillo
2012-12-21 17:01 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Manlio Perillo @ 2012-12-21 14:45 UTC (permalink / raw)
To: git
-----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-----
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: recommendation for patch maintenance
2012-12-21 14:45 recommendation for patch maintenance Manlio Perillo
@ 2012-12-21 17:01 ` Junio C Hamano
2012-12-21 17:29 ` Manlio Perillo
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2012-12-21 17:01 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git
Manlio Perillo <manlio.perillo@gmail.com> writes:
> 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.
Not offering any answer, but it is unclear to me what local changes
you are losing here. Care to explain?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: recommendation for patch maintenance
2012-12-21 17:01 ` Junio C Hamano
@ 2012-12-21 17:29 ` Manlio Perillo
2012-12-21 18:17 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Manlio Perillo @ 2012-12-21 17:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 21/12/2012 18:01, Junio C Hamano ha scritto:
> Manlio Perillo <manlio.perillo@gmail.com> writes:
>
>> 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).
>>
> [...]
>> 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.
>
> 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-----
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: recommendation for patch maintenance
2012-12-21 17:29 ` Manlio Perillo
@ 2012-12-21 18:17 ` Junio C Hamano
2012-12-21 21:30 ` Manlio Perillo
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2012-12-21 18:17 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git
Manlio Perillo <manlio.perillo@gmail.com> writes:
> 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}~4
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
$ 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 master
and 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.txt
with 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?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: recommendation for patch maintenance
2012-12-21 18:17 ` Junio C Hamano
@ 2012-12-21 21:30 ` Manlio Perillo
2012-12-21 22:17 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Manlio Perillo @ 2012-12-21 21:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 21/12/2012 19:17, Junio C Hamano ha scritto:
> [...]
> 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
>
> $ 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).
> 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:
>
> $ 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.txt
>
> with 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.
> 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?
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-----
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: recommendation for patch maintenance
2012-12-21 21:30 ` Manlio Perillo
@ 2012-12-21 22:17 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2012-12-21 22:17 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git
On Fri, Dec 21, 2012 at 1:30 PM, Manlio Perillo
<manlio.perillo@gmail.com> wrote:
>
> 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.
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-12-21 22:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-21 14:45 recommendation for patch maintenance Manlio Perillo
2012-12-21 17:01 ` Junio C Hamano
2012-12-21 17:29 ` Manlio Perillo
2012-12-21 18:17 ` Junio C Hamano
2012-12-21 21:30 ` Manlio Perillo
2012-12-21 22:17 ` Junio C Hamano
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 NNTP newsgroup(s).