Re: storing cover letter of a patch series?
From: "Duy Nguyen" On Tue, Aug 16, 2016 at 12:26 PM, Jacob Keller wrote: They can just add "squash! cover! " commits for that ;-) Though more likely the advanced workflow would be used... We'll need both (more than one) options. Or even better, "git commit --reword $SHA1" brings up the editor with commit message of $SHA1. Modify any way you want and it creates a new empty, "reword!" commit that contains the diff between the old commit message and the new one. "reword!" can be consumed by "rebase -i --autosquash" without bringing up the editor again. I realize making "git commit --reword" run multiple times would be tricky though... I was just thinking you write text and it gets appended to the text of the reworded commit, and when you squash them using rebase you get to finalize it like a normal squash? I think that's what Phillip meant by 'squash! cover!' though I wanted to go further, I don't want an editor popping up at rebase time, instead 'rebase' just update cover letter automatically for me. -- Hi Duy, While we can have code that is auto merged, I don't think that I'd want to submit a cover letter that was simply auto merged. I'd want to refresh and re-personalise the text. As long as the flexibility in our cover letter inclusion is there -- Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Mon, Aug 15, 2016 at 11:45 PM, Duy Nguyen wrote: > On Tue, Aug 16, 2016 at 12:26 PM, Jacob Keller wrote: They can just add "squash! cover! " commits for that ;-) Though more likely the advanced workflow would be used... We'll need both (more than one) options. >>> >>> Or even better, "git commit --reword $SHA1" brings up the editor with >>> commit message of $SHA1. Modify any way you want and it creates a new >>> empty, "reword!" commit that contains the diff between the old commit >>> message and the new one. "reword!" can be consumed by "rebase -i >>> --autosquash" without bringing up the editor again. I realize making >>> "git commit --reword" run multiple times would be tricky though... >> >> I was just thinking you write text and it gets appended to the text of >> the reworded commit, and when you squash them using rebase you get to >> finalize it like a normal squash? > > I think that's what Phillip meant by 'squash! cover!' though I wanted > to go further, I don't want an editor popping up at rebase time, > instead 'rebase' just update cover letter automatically for me. > -- > Duy Maybe teach it some sort of "reword! cover!" which pops up an editor and you can edit to your hearts content, and it just saves the "new" message. Since there is no such thing as a diff on message contents, it would just be a complete replace for the new message (obviously we would then strip reword and cover part out but otherwise leave the rest in place so rebase machinery would be able to fix it up without you having to edit it a second time in the rebase process? That doesn't seem as complicated as somehow storing a new diff format for the cover letter. Not sure how to handle several in a row though. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Tue, Aug 16, 2016 at 12:26 PM, Jacob Keller wrote: >>> They can just add "squash! cover! " commits for that ;-) Though more >>> likely the advanced workflow would be used... We'll need both (more than >>> one) options. >> >> Or even better, "git commit --reword $SHA1" brings up the editor with >> commit message of $SHA1. Modify any way you want and it creates a new >> empty, "reword!" commit that contains the diff between the old commit >> message and the new one. "reword!" can be consumed by "rebase -i >> --autosquash" without bringing up the editor again. I realize making >> "git commit --reword" run multiple times would be tricky though... > > I was just thinking you write text and it gets appended to the text of > the reworded commit, and when you squash them using rebase you get to > finalize it like a normal squash? I think that's what Phillip meant by 'squash! cover!' though I wanted to go further, I don't want an editor popping up at rebase time, instead 'rebase' just update cover letter automatically for me. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Mon, Aug 15, 2016 at 8:45 PM, Duy Nguyen wrote: > On Tue, Aug 16, 2016 at 3:46 AM, Philip Oakley wrote: >> From: "Jacob Keller" >> [nip] I've no problem with more extensive methods for those preparing very big patch series, or with those needing to merge together a lot of series and want to keep the cover letters, but ensuring that a simple flow is possible should still be there. -- Philip >>> >>> Some people have suggested this simple idea, and I like it, but they >>> did mention that modifying the cover letter now requires a rebase over >>> a potentially large series of patches, which can get annoying. >>> >>> Thanks, >>> Jake >> >> >> They can just add "squash! cover! " commits for that ;-) Though more >> likely the advanced workflow would be used... We'll need both (more than >> one) options. > > Or even better, "git commit --reword $SHA1" brings up the editor with > commit message of $SHA1. Modify any way you want and it creates a new > empty, "reword!" commit that contains the diff between the old commit > message and the new one. "reword!" can be consumed by "rebase -i > --autosquash" without bringing up the editor again. I realize making > "git commit --reword" run multiple times would be tricky though... > -- > Duy I was just thinking you write text and it gets appended to the text of the reworded commit, and when you squash them using rebase you get to finalize it like a normal squash? Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Tue, Aug 16, 2016 at 3:46 AM, Philip Oakley wrote: > From: "Jacob Keller" > [nip] >>> >>> >>> I've no problem with more extensive methods for those preparing very big >>> patch series, or with those needing to merge together a lot of series and >>> want to keep the cover letters, but ensuring that a simple flow is >>> possible >>> should still be there. >>> -- >>> Philip >>> >> >> Some people have suggested this simple idea, and I like it, but they >> did mention that modifying the cover letter now requires a rebase over >> a potentially large series of patches, which can get annoying. >> >> Thanks, >> Jake > > > They can just add "squash! cover! " commits for that ;-) Though more > likely the advanced workflow would be used... We'll need both (more than > one) options. Or even better, "git commit --reword $SHA1" brings up the editor with commit message of $SHA1. Modify any way you want and it creates a new empty, "reword!" commit that contains the diff between the old commit message and the new one. "reword!" can be consumed by "rebase -i --autosquash" without bringing up the editor again. I realize making "git commit --reword" run multiple times would be tricky though... -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Mon, Aug 15, 2016 at 1:38 PM, Junio C Hamano wrote: > Jacob Keller writes: > >> Some people have suggested this simple idea, and I like it, but they >> did mention that modifying the cover letter now requires a rebase over >> a potentially large series of patches, which can get annoying. > > That can be simply solved by keeping the cover at the end. When you > are updating the real patch on the series with "rebase -i", you > would have a chance to update the cover at the same time that way. > It has problems keeping it at the end as well because that makes regular commits and commit --amend funky.., but you could do squashes into the cover letter easily enough as suggested by Philip Oakley. I think that might be the most natural flow we have now that doesn't depend on creating some fancy new object type. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
From: "Jacob Keller" [nip] I've no problem with more extensive methods for those preparing very big patch series, or with those needing to merge together a lot of series and want to keep the cover letters, but ensuring that a simple flow is possible should still be there. -- Philip Some people have suggested this simple idea, and I like it, but they did mention that modifying the cover letter now requires a rebase over a potentially large series of patches, which can get annoying. Thanks, Jake They can just add "squash! cover! " commits for that ;-) Though more likely the advanced workflow would be used... We'll need both (more than one) options. -- Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Jacob Keller writes: > Some people have suggested this simple idea, and I like it, but they > did mention that modifying the cover letter now requires a rebase over > a potentially large series of patches, which can get annoying. That can be simply solved by keeping the cover at the end. When you are updating the real patch on the series with "rebase -i", you would have a chance to update the cover at the same time that way. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Mon, Aug 15, 2016 at 5:37 AM, Philip Oakley wrote: > [sorry if this is not the right place to 'drop in'..] > I appreciate there has been a lot of discussion, but it mainly appears to be > about an upstream / integration viewpoint. > > I'd hate it if there was a one size fits all solution that was only focused > on one important use case, rather than having at least a simple fallback for > simple folk. > > Personally I liked the idea that I could start my patch series branch with a > simple 'empty' commit with a commit message that read "cover! the series>" and continue with the cover letter. It's essentially the same > as the fixup! and squash! idea (more the latter - it's squash! without a > predecessor). For moderate size series a simple 'git rebase master..' is > sufficient to see the whole series and decide which need editing, rewording, > swapping, checking the fixups, etc. > > Format-patch would then be taught to spot that the first commit in the > series is "cover! " and create the usual 0/N cover letter. Git Gui > may need to be taught to recognise cover! (haven't checked if it recognises > an empty commit squash!). Possibly 'git commit' may want a --cover option to > massage the commit message and add --allow-empty, but that's finesse. > > I've no problem with more extensive methods for those preparing very big > patch series, or with those needing to merge together a lot of series and > want to keep the cover letters, but ensuring that a simple flow is possible > should still be there. > -- > Philip > Some people have suggested this simple idea, and I like it, but they did mention that modifying the cover letter now requires a rebase over a potentially large series of patches, which can get annoying. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Mon, Aug 15, 2016 at 08:30:03PM +0700, Duy Nguyen wrote: > On Mon, Aug 15, 2016 at 7:37 PM, Philip Oakley wrote: > > I appreciate there has been a lot of discussion, but it mainly appears to be > > about an upstream / integration viewpoint. > > > > I'd hate it if there was a one size fits all solution that was only focused > > on one important use case, rather than having at least a simple fallback for > > simple folk. > > > > Personally I liked the idea that I could start my patch series branch with a > > simple 'empty' commit with a commit message that read "cover! > the series>" and continue with the cover letter. It's essentially the same > > as the fixup! and squash! idea (more the latter - it's squash! without a > > predecessor). For moderate size series a simple 'git rebase master..' is > > sufficient to see the whole series and decide which need editing, rewording, > > swapping, checking the fixups, etc. > > I think you hit the jackpot (or are getting very close). This removes > the special status of "the commit at the tip of the branch" cover > letter. Maybe I just like it so much I have a hard time finding > anything wrong with it :) I haven't followed this thread too closely, but has anyone mentioned U-Boot's patman tool[1] yet? It defines several special trailers that can be used to annotate commits with additional information to use when mailing them and which are automatically removed from the commit message in patches sent using patman. [1] http://git.denx.de/?p=u-boot.git;a=blob;f=tools/patman/README -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Mon, Aug 15, 2016 at 7:37 PM, Philip Oakley wrote: > I appreciate there has been a lot of discussion, but it mainly appears to be > about an upstream / integration viewpoint. > > I'd hate it if there was a one size fits all solution that was only focused > on one important use case, rather than having at least a simple fallback for > simple folk. > > Personally I liked the idea that I could start my patch series branch with a > simple 'empty' commit with a commit message that read "cover! the series>" and continue with the cover letter. It's essentially the same > as the fixup! and squash! idea (more the latter - it's squash! without a > predecessor). For moderate size series a simple 'git rebase master..' is > sufficient to see the whole series and decide which need editing, rewording, > swapping, checking the fixups, etc. I think you hit the jackpot (or are getting very close). This removes the special status of "the commit at the tip of the branch" cover letter. Maybe I just like it so much I have a hard time finding anything wrong with it :) > Format-patch would then be taught to spot that the first commit in the > series is "cover! " and create the usual 0/N cover letter. Git Gui > may need to be taught to recognise cover! (haven't checked if it recognises > an empty commit squash!). Possibly 'git commit' may want a --cover option to > massage the commit message and add --allow-empty, but that's finesse. > > I've no problem with more extensive methods for those preparing very big > patch series, or with those needing to merge together a lot of series and > want to keep the cover letters, but ensuring that a simple flow is possible > should still be there. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
From: "Duy Nguyen" On Mon, Aug 15, 2016 at 1:28 PM, Stefan Beller wrote: On Sun, Aug 14, 2016 at 12:15 AM, Jacob Keller wrote: On Sat, Aug 13, 2016 at 1:49 AM, Duy Nguyen wrote: On Tue, Aug 9, 2016 at 12:27 AM, Stefan Beller wrote: is what you want. Maybe we want to see a patch that adds the reverse functionality as well, i.e. git-am will store the the cover letter as the branch description and git-merge will propose the branch description for the merge commit. I almost suggested the same, but there is a problem with this approach: if you're are on a detached head, where does git-am save it? What would the user expect? We can have a range of expectations: 1) reject and error out git-am 2) warn about not saving branch.description and continue with am 3) have a (maybe special) branch.HEAD.description thing, same for FETCH_HEAD etc 4) have a config option to choose between 1 and 2, if unset default to 1 I think 3 is a bad choice. 4 seems reasonable to me, though I wonder if some people use git-am in a scripted workflow with a detached head and then create the branch afterwards? So 5) create a branch for them? (such as $(date)-${subject}) My gut reaction doesn't like 5 either. I'm starting to think option 6 (storing cover latter as an empty commit at tip then git-merge replaces it with a merge commit in a permanent history) may be the way to go. It handles detached heads just fine, we have reflog to store older cover letters. Though it will not play nice with 'git commit --amend' and 'git reset' for people who rewrites history heavily during development, but maybe 'git rebase -i --autosquash' would be an ok workflow alternative. -- [sorry if this is not the right place to 'drop in'..] I appreciate there has been a lot of discussion, but it mainly appears to be about an upstream / integration viewpoint. I'd hate it if there was a one size fits all solution that was only focused on one important use case, rather than having at least a simple fallback for simple folk. Personally I liked the idea that I could start my patch series branch with a simple 'empty' commit with a commit message that read "cover! " and continue with the cover letter. It's essentially the same as the fixup! and squash! idea (more the latter - it's squash! without a predecessor). For moderate size series a simple 'git rebase master..' is sufficient to see the whole series and decide which need editing, rewording, swapping, checking the fixups, etc. Format-patch would then be taught to spot that the first commit in the series is "cover! " and create the usual 0/N cover letter. Git Gui may need to be taught to recognise cover! (haven't checked if it recognises an empty commit squash!). Possibly 'git commit' may want a --cover option to massage the commit message and add --allow-empty, but that's finesse. I've no problem with more extensive methods for those preparing very big patch series, or with those needing to merge together a lot of series and want to keep the cover letters, but ensuring that a simple flow is possible should still be there. -- Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Mon, Aug 15, 2016 at 1:28 PM, Stefan Beller wrote: > On Sun, Aug 14, 2016 at 12:15 AM, Jacob Keller wrote: >> On Sat, Aug 13, 2016 at 1:49 AM, Duy Nguyen wrote: >>> On Tue, Aug 9, 2016 at 12:27 AM, Stefan Beller wrote: is what you want. Maybe we want to see a patch that adds the reverse functionality as well, i.e. git-am will store the the cover letter as the branch description and git-merge will propose the branch description for the merge commit. >>> >>> I almost suggested the same, but there is a problem with this >>> approach: if you're are on a detached head, where does git-am save it? > > What would the user expect? We can have a range of expectations: > 1) reject and error out git-am > 2) warn about not saving branch.description and continue with am > 3) have a (maybe special) branch.HEAD.description thing, same for FETCH_HEAD > etc > 4) have a config option to choose between 1 and 2, if unset default to 1 > > I think 3 is a bad choice. > 4 seems reasonable to me, though I wonder if some people use git-am in > a scripted workflow with a detached head and then create the branch > afterwards? > So > > 5) create a branch for them? (such as $(date)-${subject}) > > My gut reaction doesn't like 5 either. I'm starting to think option 6 (storing cover latter as an empty commit at tip then git-merge replaces it with a merge commit in a permanent history) may be the way to go. It handles detached heads just fine, we have reflog to store older cover letters. Though it will not play nice with 'git commit --amend' and 'git reset' for people who rewrites history heavily during development, but maybe 'git rebase -i --autosquash' would be an ok workflow alternative. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Sun, Aug 14, 2016 at 11:49 PM, Stefan Beller wrote: > On Sun, Aug 14, 2016 at 11:38 PM, Jacob Keller wrote: >> On Sun, Aug 14, 2016 at 11:28 PM, Stefan Beller wrote: >>> I would imagine this is similar to the pull requests on the linux >>> mailing list, i.e. >>> how it is with merges. Back in the time we did not open the editor for you >>> to >>> talk about the merge you just did, and then we started doing that. >>> >>> So what to do when the description already exists? >>> >>> We could amend the description separated by a >>> >>> # comment, below was added: >>> >>> line or such and then open the editor asked for user input. >>> >>> Thanks, >>> Stefan >>> >> >> This is why my gut feeling is that we should instead have a separate >> way to store a cover letter, as it doesn't necessarily have to apply >> to a branch > > Well in our workflow each series has at least one merge commit. > (You *could* have different descriptions for the different branches, > e.g. for maint: "fixes a segfault so let's get this in, but it needs to be > redone properly" and for pu: "TODO: revert this partially > when branch $proper-fix is merged") > >> or a merge commit, but could just be annotation against a >> series of commits (maybe stored as base + tip, since most series would >> be linear in nature?) > > We could suggest to use a merge always strategy for this, i.e. as soon as > you send a cover-letter, we'll make a merge for you whose parents are the > old HEAD and the new series? > > If the user strictly wants to have a linear history, then we could try some > empty commit magic before or after the series, but I doubt this is proper. > > If users insist on linear history, they deny the benefits of a DAG that > represents how the source code evolved. (Also see the eternal rebase > vs merge discussion ;) > I think you're right this can go into a merge commit and if a user insists on linear history it's their fault. >> >> However, opening an editor and amending seems quite reasonable to me >> if we're just editing branch description, and then storing that as >> part of merge commit would be reasonable? >> >> I really think we want some alternative way to store it for other use >> cases besides the description, though. > > "besides the description"? I think my brain shut down. I'm not really sure what I meant but I think I meant "we want some other way to make the cover letter permanent because the branch description isn't shared" So... no I have no real idea what I was trying to say here. Thanks, Jake > > What do you mean by that? > > Thanks, > Stefan > >> >> Regards, >> Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Sun, Aug 14, 2016 at 11:38 PM, Jacob Keller wrote: > On Sun, Aug 14, 2016 at 11:28 PM, Stefan Beller wrote: >> I would imagine this is similar to the pull requests on the linux >> mailing list, i.e. >> how it is with merges. Back in the time we did not open the editor for you to >> talk about the merge you just did, and then we started doing that. >> >> So what to do when the description already exists? >> >> We could amend the description separated by a >> >> # comment, below was added: >> >> line or such and then open the editor asked for user input. >> >> Thanks, >> Stefan >> > > This is why my gut feeling is that we should instead have a separate > way to store a cover letter, as it doesn't necessarily have to apply > to a branch Well in our workflow each series has at least one merge commit. (You *could* have different descriptions for the different branches, e.g. for maint: "fixes a segfault so let's get this in, but it needs to be redone properly" and for pu: "TODO: revert this partially when branch $proper-fix is merged") > or a merge commit, but could just be annotation against a > series of commits (maybe stored as base + tip, since most series would > be linear in nature?) We could suggest to use a merge always strategy for this, i.e. as soon as you send a cover-letter, we'll make a merge for you whose parents are the old HEAD and the new series? If the user strictly wants to have a linear history, then we could try some empty commit magic before or after the series, but I doubt this is proper. If users insist on linear history, they deny the benefits of a DAG that represents how the source code evolved. (Also see the eternal rebase vs merge discussion ;) > > However, opening an editor and amending seems quite reasonable to me > if we're just editing branch description, and then storing that as > part of merge commit would be reasonable? > > I really think we want some alternative way to store it for other use > cases besides the description, though. "besides the description"? What do you mean by that? Thanks, Stefan > > Regards, > Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Sun, Aug 14, 2016 at 11:28 PM, Stefan Beller wrote: > I would imagine this is similar to the pull requests on the linux > mailing list, i.e. > how it is with merges. Back in the time we did not open the editor for you to > talk about the merge you just did, and then we started doing that. > > So what to do when the description already exists? > > We could amend the description separated by a > > # comment, below was added: > > line or such and then open the editor asked for user input. > > Thanks, > Stefan > This is why my gut feeling is that we should instead have a separate way to store a cover letter, as it doesn't necessarily have to apply to a branch or a merge commit, but could just be annotation against a series of commits (maybe stored as base + tip, since most series would be linear in nature?) However, opening an editor and amending seems quite reasonable to me if we're just editing branch description, and then storing that as part of merge commit would be reasonable? I really think we want some alternative way to store it for other use cases besides the description, though. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Sun, Aug 14, 2016 at 12:15 AM, Jacob Keller wrote: > On Sat, Aug 13, 2016 at 1:49 AM, Duy Nguyen wrote: >> On Tue, Aug 9, 2016 at 12:27 AM, Stefan Beller wrote: >>> is what you want. Maybe we want to see a patch that adds the reverse >>> functionality as well, i.e. git-am will store the the cover letter as the >>> branch description and git-merge will propose the branch description for >>> the merge commit. >> >> I almost suggested the same, but there is a problem with this >> approach: if you're are on a detached head, where does git-am save it? What would the user expect? We can have a range of expectations: 1) reject and error out git-am 2) warn about not saving branch.description and continue with am 3) have a (maybe special) branch.HEAD.description thing, same for FETCH_HEAD etc 4) have a config option to choose between 1 and 2, if unset default to 1 I think 3 is a bad choice. 4 seems reasonable to me, though I wonder if some people use git-am in a scripted workflow with a detached head and then create the branch afterwards? So 5) create a branch for them? (such as $(date)-${subject}) My gut reaction doesn't like 5 either. >> -- >> Duy > > Also, what about the case where a branch already has a description > such as is the case for something other than an integration branch. > How does git-am know the difference and ensure it doesn't overwrite > anything? Not everyone uses separate branches for each patch and such. I would imagine this is similar to the pull requests on the linux mailing list, i.e. how it is with merges. Back in the time we did not open the editor for you to talk about the merge you just did, and then we started doing that. So what to do when the description already exists? We could amend the description separated by a # comment, below was added: line or such and then open the editor asked for user input. Thanks, Stefan > > Thanks, > Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Sat, Aug 13, 2016 at 1:49 AM, Duy Nguyen wrote: > On Tue, Aug 9, 2016 at 12:27 AM, Stefan Beller wrote: >> is what you want. Maybe we want to see a patch that adds the reverse >> functionality as well, i.e. git-am will store the the cover letter as the >> branch description and git-merge will propose the branch description for >> the merge commit. > > I almost suggested the same, but there is a problem with this > approach: if you're are on a detached head, where does git-am save it? > -- > Duy Also, what about the case where a branch already has a description such as is the case for something other than an integration branch. How does git-am know the difference and ensure it doesn't overwrite anything? Not everyone uses separate branches for each patch and such. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Tue, Aug 9, 2016 at 12:27 AM, Stefan Beller wrote: > On Thu, Sep 10, 2015 at 9:28 AM, Jacob Keller wrote: >> Hey, >> >> does anyone know of any tricks for storing a cover letter for a patch >> series inside of git somehow? I'd guess the only obvious way currently >> is to store it at the top of the series as an empty commit.. but this >> doesn't get emailed *as* the cover letter... >> >> Is there some other way? Would others be interested in such a feature? > > Being late to this thread, but I think > >branch..description >Branch description, can be edited with git branch >--edit-description. Branch description is automatically added in >the format-patch cover letter or request-pull summary. > > is what you want. Maybe we want to see a patch that adds the reverse > functionality as well, i.e. git-am will store the the cover letter as the > branch description and git-merge will propose the branch description for > the merge commit. I almost suggested the same, but there is a problem with this approach: if you're are on a detached head, where does git-am save it? -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Junio C Hamano venit, vidit, dixit 08.08.2016 19:42: > Duy Nguyen writes: > >> git-notes was mentioned in this thread back in 2015, but I think it's >> discarded because of the argument that's part of the cover letter was >> not meant to be kept permanently. > > I do not think the reason why we didn't think the notes mechanism > was a good match was mainly because the cover letter material was > about a branch as a whole, which does not have a good counter-part > in Git at the conceptual level. "A branch is just a moving pointer > that points at one commit that happens to be at the tip" is not a > perfect match to "I am holding these N patches to achieve X and I am > constantly adding, rewinding and rebuilding". The notes mechanism > gives an easy way to describe the former (i.e. annotate one commit, > and let various commands to move that notes as you rewind and > rebuild) but not the latter (i.e. "branch.description" configuration > is the best match, but that is just a check-box feature and does not > make any serious attempt to be part of a version-control system). > >> But I think we can still use it as a >> local/temporary place for cover letter instead of the empty commit at >> the topic's tip. It is a mark of the beginning of commit, it does not >> require rewriting history when you update the cover letter, and >> git-merge can be taught to pick it up when you're ready to set it in >> stone. > > That depends on what you exactly mean by "the beginning of". Do you > mean the first commit that is on the topic? Then that still requires > you to move it around when the topic is rebuilt. If you mean the > commit on the mainline the topic forks from, then of course that > would not work, as you can fork multiple topics at the same commit. Well, my idea back then was: attach notes to refs rather than commits, in this case to the branch ref. That would solve both the "branch moves" as well as the "cover letter refers to the whole branch/topic" issues. In fact, I had an implementation that I had been rebasing and using for quite some time, but it never became popular, and branch.description landed in-tree. [short version: notes attached to (virtual) refname objects (virtual blobs - not stored, but "existing" for (fsck, notes prune and the like)] The notes based approach suffered from the old notes deficiency: we don't have good simple tooling for sharing notes; really, we don't have good tooling for dealing with any remote refs besides branches (read: ref namespace reorg project is stalled), unless you set up specific refspecs. OTOH, branch.description is inherently local, too, and can't even be transported after setting up refspecs or such. Also, notes trees have a history, so you would gain a log on your cover letter edits; again, our tooling around that notes feature is sub-optimal, that is: the feature is there, the ui could improve. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Stefan Beller writes: > On Thu, Sep 10, 2015 at 9:28 AM, Jacob Keller wrote: >> Hey, >> >> does anyone know of any tricks for storing a cover letter for a patch >> series inside of git somehow? I'd guess the only obvious way currently >> is to store it at the top of the series as an empty commit.. but this >> doesn't get emailed *as* the cover letter... >> >> Is there some other way? Would others be interested in such a feature? > > Being late to this thread, but I think > >branch..description >Branch description, can be edited with git branch >--edit-description. Branch description is automatically added in >the format-patch cover letter or request-pull summary. > > is what you want. Maybe we want to see a patch that adds the reverse > functionality as well, i.e. git-am will store the the cover letter as the > branch description and git-merge will propose the branch description for > the merge commit. Yes, but... ;-) It is a bit too weak to be called a proper part of a "version control system", in that the description, even though it can be edited with "--edit-description", is not versioned. It is consistent with the fact that rerolls of your branch by rebuilding with "rebase -i" or "checkout --detached && until satisified; do cherry-pick && commit --amend; done" is not versioned and you may resort to the old-fashioned my-topic, my-topic-v2, my-topic-v3, ..., but being consistent with a bad part of the system does not deny the fact that it is a weak feature. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Duy Nguyen writes: > git-notes was mentioned in this thread back in 2015, but I think it's > discarded because of the argument that's part of the cover letter was > not meant to be kept permanently. I do not think the reason why we didn't think the notes mechanism was a good match was mainly because the cover letter material was about a branch as a whole, which does not have a good counter-part in Git at the conceptual level. "A branch is just a moving pointer that points at one commit that happens to be at the tip" is not a perfect match to "I am holding these N patches to achieve X and I am constantly adding, rewinding and rebuilding". The notes mechanism gives an easy way to describe the former (i.e. annotate one commit, and let various commands to move that notes as you rewind and rebuild) but not the latter (i.e. "branch.description" configuration is the best match, but that is just a check-box feature and does not make any serious attempt to be part of a version-control system). > But I think we can still use it as a > local/temporary place for cover letter instead of the empty commit at > the topic's tip. It is a mark of the beginning of commit, it does not > require rewriting history when you update the cover letter, and > git-merge can be taught to pick it up when you're ready to set it in > stone. That depends on what you exactly mean by "the beginning of". Do you mean the first commit that is on the topic? Then that still requires you to move it around when the topic is rebuilt. If you mean the commit on the mainline the topic forks from, then of course that would not work, as you can fork multiple topics at the same commit. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Thu, Sep 10, 2015 at 9:28 AM, Jacob Keller wrote: > Hey, > > does anyone know of any tricks for storing a cover letter for a patch > series inside of git somehow? I'd guess the only obvious way currently > is to store it at the top of the series as an empty commit.. but this > doesn't get emailed *as* the cover letter... > > Is there some other way? Would others be interested in such a feature? Being late to this thread, but I think branch..description Branch description, can be edited with git branch --edit-description. Branch description is automatically added in the format-patch cover letter or request-pull summary. is what you want. Maybe we want to see a patch that adds the reverse functionality as well, i.e. git-am will store the the cover letter as the branch description and git-merge will propose the branch description for the merge commit. > > I get very annoyed when I've written a nice long patch cover letter in > vim before an email and then realize I should fix something else up, > or accidentally cancel it because I didn't use the write "To:" address > or something.. > > I really think it should be possible to store something somehow as a > blob that could be looked up later. Even if this was a slightly more > manual process that would be helpful to store the message inside git > itself. I agree here. I personally do not use that variable (yet), as it doesn't seem to be editable easily. So here is what I do: 1) First series is generated with format-patch --cover-letter 2) any following v{2,3,4} is generated without the cover-letter but with --subject-prefix=PATCHv{2,3,4} 3) the cover letter is manually edited to be the next version and a section is added why the next version of the series is sent. > > In addition, this would help re-rolls since it would mean if I go back > to a topic and re-roll it I can just update the message. If it were > properly stored in my local history that would also mean I could see > revisions on it. > > Any thoughts on how to do this? > > Regards, > Jake > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Sun, Aug 7, 2016 at 7:12 AM, Michael S. Tsirkin wrote: > On Fri, Aug 05, 2016 at 08:39:58AM -0700, Junio C Hamano wrote: >> "Michael S. Tsirkin" writes: >> >> > On Thu, Sep 10, 2015 at 11:39:49AM -0700, Junio C Hamano wrote: >> >> The problem with "empty commit trick" is that it is a commit whose >> >> sole purpose is to describe the series, and its presence makes it >> >> clear where the series ends, but the topology does not tell where >> >> the series begins, so it is an unsatisifactory half-measure. >> > >> > Actually, when using topic branches the series always ends at head, so >> > it's better to keep the empty commit where series begins. >> >> But that would mean that you would need to destroy and recreate more >> commits than you would need to. If you have a five-commit series >> (with the bottom "description" one, you would have six commits) and >> you are already happy with the bottom two but want to update the >> third one, you wuld have to "rebase -i" all six of them, reword the >> bottom "description" to adjust it to describe the new version of the >> third one _before_ you even do the actual update of the third one. >> >> That somehow feels backwards, and that backward-ness comes from the >> fact that you abused a single-parent commit for the purpose it is >> not meant to be used (i.e. they are to describe individual changes), >> because you did not find a better existing mechanism (and I suspect >> there isn't any, in which case the solution is to invent one, not >> abusing an existing mechanism that is not suited for it). > > A flag that marks a commit "beginning of series" then? git-notes was mentioned in this thread back in 2015, but I think it's discarded because of the argument that's part of the cover letter was not meant to be kept permanently. But I think we can still use it as a local/temporary place for cover letter instead of the empty commit at the topic's tip. It is a mark of the beginning of commit, it does not require rewriting history when you update the cover letter, and git-merge can be taught to pick it up when you're ready to set it in stone. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Sun, Aug 07, 2016 at 08:12:23AM +0300, Michael S. Tsirkin wrote: > On Fri, Aug 05, 2016 at 08:39:58AM -0700, Junio C Hamano wrote: > > * When you updated an existing topic, you tell a tool like "rebase > >-i -p" to recreate "lit" branch on top of the mainline. This > >would give you an opportunity to update the cover. > > Combining patchsets might need conflict resolution, > redoing this each time might be a lot of work. git-rerere can generally handle that pretty well. I wrote a tool [1] to manage integration branches which I use pretty heavily and I find it very rare to hit a serious conflict. In fact, git-integration has an "autocontinue" mode which accepts git-rerere's resolution if it has one, which works reliably in my experience. I hadn't thought about writing the cover letter in the integration branch instruction sheet (I normally just put in some notes for myself about the state of the branch), but I suspect it would be quite easy to write a script that mails a series using the instruction sheet comments as the cover letter. [1] http://johnkeeping.github.io/git-integration/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Fri, Aug 05, 2016 at 08:39:58AM -0700, Junio C Hamano wrote: > "Michael S. Tsirkin" writes: > > > On Thu, Sep 10, 2015 at 11:39:49AM -0700, Junio C Hamano wrote: > >> The problem with "empty commit trick" is that it is a commit whose > >> sole purpose is to describe the series, and its presence makes it > >> clear where the series ends, but the topology does not tell where > >> the series begins, so it is an unsatisifactory half-measure. > > > > Actually, when using topic branches the series always ends at head, so > > it's better to keep the empty commit where series begins. > > But that would mean that you would need to destroy and recreate more > commits than you would need to. If you have a five-commit series > (with the bottom "description" one, you would have six commits) and > you are already happy with the bottom two but want to update the > third one, you wuld have to "rebase -i" all six of them, reword the > bottom "description" to adjust it to describe the new version of the > third one _before_ you even do the actual update of the third one. > > That somehow feels backwards, and that backward-ness comes from the > fact that you abused a single-parent commit for the purpose it is > not meant to be used (i.e. they are to describe individual changes), > because you did not find a better existing mechanism (and I suspect > there isn't any, in which case the solution is to invent one, not > abusing an existing mechanism that is not suited for it). A flag that marks a commit "beginning of series" then? > If this were part of a workflow like this, I would understand it: > > * Build a N-commit series on a topic. > > * You keep a "local integration testing" branch ("lit"), forked >from a mainline and updated _every time_ you do something to your >topics. You may or may not publish this branch. This is the >aggregation of what you locally have done, a convenient place to >test individual topics together before they get published. This seems to assume topic branches. I know you use them, but not overyone does, I don't. > * A new topic, when you merge it to the "lit" branch, you describe >the cover as the merge commit message. > > * When you updated an existing topic, you tell a tool like "rebase >-i -p" to recreate "lit" branch on top of the mainline. This >would give you an opportunity to update the cover. Combining patchsets might need conflict resolution, redoing this each time might be a lot of work. > Now the tool support for the last one is the missing piece. In > addition to what "rebase -i -p" would, it at least need to > automatically figure out which topics have been updated, so that > their merge commit log messages need to be given in the editor to > update, while carrying over the merge log message for other topics > intact (by default). > > With that, you should also be able to teach "format-patch --cover" > to take these merge messages on "lit" into account when it creates > the cover letter. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Junio C Hamano writes: > On Fri, Aug 5, 2016 at 2:20 PM, Martin Fick wrote: >> On Friday, August 05, 2016 08:39:58 AM you wrote: >>> * A new topic, when you merge it to the "lit" branch, you >>> describe the cover as the merge commit message. >>> >>> * When you updated an existing topic, you tell a tool >>> like "rebase -i -p" to recreate "lit" branch on top of >>> the mainline. This would give you an opportunity to >>> update the cover. >> >> This is a neat idea. How would this work if there is no >> merge commit (mainline hasn't moved)? > > Sorry, I do not understand your question. You always > merge into your own "lit", which is based on (some) > version of the mainline. If a topic builds on top of the > mainline, you "merge --no-ff" it into "lit". Because no > merges on "lit" will be part of the future mainline anyway, > even the project frowns upon a "no-ff" merge, that will > not be a problem. In any case, the "if you want to say more than what the individual commits say about the topic as a whole, say it in the merge that brings them all into an integration branch" is not just "a neat idea". Recent versions of Git actively _encourages_ you to describe what it is about by opening your editor when you create a merge, and the cover letter material is something you would want the merge of your topic into the upstream to say when your topic finally lands there. And as the author of a topic, the person who writes the cover letter is well qualified to describe what the topic as a whole is about, how it relates to the state of the entire project before that merge happens. That is what you want to write in the cover letter. So "write it in a merge log message yourself, and somehow find a way to propagate it to the maintainer's tree" is the natural consequence of thinking and working backwards from what we want to have in the final history; not any novel (or neat) idea. What follows is that at the receiving end (i.e. "git am") it may be suboptimal to create an empty commit to record the cover letter material. Storing at the bottom of the received pile of commits is out of question. It _might_ be acceptable to queue it as the tip, and then teach "git merge $topic" to notice that $topic^0 is such a "cover letter commit", and turn itself into "git merge $topic^1 && git commit --amend -C $topic", though. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Fri, Aug 5, 2016 at 2:20 PM, Martin Fick wrote: > On Friday, August 05, 2016 08:39:58 AM you wrote: >> * A new topic, when you merge it to the "lit" branch, you >> describe the cover as the merge commit message. >> >> * When you updated an existing topic, you tell a tool >> like "rebase -i -p" to recreate "lit" branch on top of >> the mainline. This would give you an opportunity to >> update the cover. > > This is a neat idea. How would this work if there is no > merge commit (mainline hasn't moved)? Sorry, I do not understand your question. You always merge into your own "lit", which is based on (some) version of the mainline. If a topic builds on top of the mainline, you "merge --no-ff" it into "lit". Because no merges on "lit" will be part of the future mainline anyway, even the project frowns upon a "no-ff" merge, that will not be a problem. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Friday, August 05, 2016 08:39:58 AM you wrote: > * A new topic, when you merge it to the "lit" branch, you > describe the cover as the merge commit message. > > * When you updated an existing topic, you tell a tool > like "rebase -i -p" to recreate "lit" branch on top of > the mainline. This would give you an opportunity to > update the cover. This is a neat idea. How would this work if there is no merge commit (mainline hasn't moved)? -Martin -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
"Michael S. Tsirkin" writes: > On Thu, Sep 10, 2015 at 11:39:49AM -0700, Junio C Hamano wrote: >> The problem with "empty commit trick" is that it is a commit whose >> sole purpose is to describe the series, and its presence makes it >> clear where the series ends, but the topology does not tell where >> the series begins, so it is an unsatisifactory half-measure. > > Actually, when using topic branches the series always ends at head, so > it's better to keep the empty commit where series begins. But that would mean that you would need to destroy and recreate more commits than you would need to. If you have a five-commit series (with the bottom "description" one, you would have six commits) and you are already happy with the bottom two but want to update the third one, you wuld have to "rebase -i" all six of them, reword the bottom "description" to adjust it to describe the new version of the third one _before_ you even do the actual update of the third one. That somehow feels backwards, and that backward-ness comes from the fact that you abused a single-parent commit for the purpose it is not meant to be used (i.e. they are to describe individual changes), because you did not find a better existing mechanism (and I suspect there isn't any, in which case the solution is to invent one, not abusing an existing mechanism that is not suited for it). If this were part of a workflow like this, I would understand it: * Build a N-commit series on a topic. * You keep a "local integration testing" branch ("lit"), forked from a mainline and updated _every time_ you do something to your topics. You may or may not publish this branch. This is the aggregation of what you locally have done, a convenient place to test individual topics together before they get published. * A new topic, when you merge it to the "lit" branch, you describe the cover as the merge commit message. * When you updated an existing topic, you tell a tool like "rebase -i -p" to recreate "lit" branch on top of the mainline. This would give you an opportunity to update the cover. Now the tool support for the last one is the missing piece. In addition to what "rebase -i -p" would, it at least need to automatically figure out which topics have been updated, so that their merge commit log messages need to be given in the editor to update, while carrying over the merge log message for other topics intact (by default). With that, you should also be able to teach "format-patch --cover" to take these merge messages on "lit" into account when it creates the cover letter. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Thu, Sep 10, 2015 at 11:39:49AM -0700, Junio C Hamano wrote: > The problem with "empty commit trick" is that it is a commit whose > sole purpose is to describe the series, and its presence makes it > clear where the series ends, but the topology does not tell where > the series begins, so it is an unsatisifactory half-measure. Actually, when using topic branches the series always ends at head, so it's better to keep the empty commit where series begins. This was actually suggested by Philip Oakley on this thread but I'm not sure it was noticed as it was part of a bigger email. It also maps much better to git am uses - you apply patch 0/N first to create the empty commit, then the rest of the patches. This does mean you need to use git rebase to edit that cover commit, but maybe that is not a big deal, and git rebase could learn --cover to find and edit that. -- MST -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Thu, Sep 10, 2015 at 02:03:48PM -0700, Jacob Keller wrote: > On Thu, Sep 10, 2015 at 1:09 PM, Philip Oakley wrote: > > From: "Jacob Keller" > >> > >> On Thu, Sep 10, 2015 at 11:44 AM, Junio C Hamano > >> wrote: > >>> > >>> Jacob Keller writes: > >>> > I hadn't thought of separating the cover letter from git-send-email. > That would be suitable for me. > >>> > >>> > >>> Yeah, I said this number of times over time, and I said it once > >>> recently in another thread, but I think it was a mistake to allow > >>> git-send-email to drive format-patch. It may appear that it will > >>> make things convenient in the perfect world where no user makes > >>> mistakes, but people are not perfect in real life. Expecting them > >>> to be is being naive. > >>> > >> > >> Yep. I didn't even know cover-letter was an option of format-patch > >> only thought it was in send-email. > >> > > Actually, the one feature I'd like (I think) is to be able to join together > > the empty commit mechanism and the cover letter mechanism within format > > patch so that: > > > > * the empty commit message would detected and automatically become the [0/N] > > in the patch series (without need to say --cover-letter) > > > > * the cover letter would still have some 'template' markings to say "*** > > insert what's changed here***" or smilar (with option to exclude them). > > > > That way, when starting a series / branch, the first item would be to add > > the explanatory 'empty commit' that states the requirements of what one > > hopes to achieve (a key cover letter content), which is then followed by > > commits that move toward that goal. > > > > The series can then be rebased as the user develops the code, and that cover > > note can be edited as required during the rebase. > > > > When it comes time to show it to the list, the format patch will *know* from > > the empty commit that it is the [0/N] cover letter and (perhaps -option) add > > the appropriate markers ready for editing. And perhaps git am could learn an option to apply 0/N as a cover commit. > > The user edits the cover letter with the extra 'what's changed' / interdiff > > / whatever, and sends. sendmail barfs if the user hasn't edited the markers. > > > > This could also work with the sendmail patch formating (though I've never > > used that workflow) as now the cover letter becomes automatic for the > > upstream. > > > > Philip > > If there was a way to store this empty commit message tagged as "cover > letter" that could work well, though generally I prefer the > non-fast-forward merges as this shows you where the series ended *and* > began. It's somewhat confusing to newer users.. and this doesn't get > rebased very well either. > > Some way to indicate a particular "empty" commit is actually a cover > letter seems easy enough. This seems like the way that I was thinking. Start the subject with "cover! "? I have a patch that teaches git-rebase to keep empty commits where the subject has a given prefix, that might be helpful there. > Using "edit description" of git-branch seems also to be pretty > effective for this, even if it doesn't get shared across remotes. (not > really a necessary feature for what I do). > > But having some way to indicate "cover letter" which gets used as the > beginning of a log message when doing a particular "merge > --tip-as-cover" or something like Junio suggested above seems like the > nicest approach. > > Regards, > Jake > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Hi Jacob, On 11 September 2015 at 02:30, Chris Packham wrote: > On Fri, Sep 11, 2015 at 4:28 AM, Jacob Keller wrote: >> Hey, >> >> does anyone know of any tricks for storing a cover letter for a patch >> series inside of git somehow? I'd guess the only obvious way currently >> is to store it at the top of the series as an empty commit.. but this >> doesn't get emailed *as* the cover letter... >> >> Is there some other way? Would others be interested in such a feature? >> >> I get very annoyed when I've written a nice long patch cover letter in >> vim before an email and then realize I should fix something else up, >> or accidentally cancel it because I didn't use the write "To:" address >> or something.. >> >> I really think it should be possible to store something somehow as a >> blob that could be looked up later. Even if this was a slightly more >> manual process that would be helpful to store the message inside git >> itself. >> >> In addition, this would help re-rolls since it would mean if I go back >> to a topic and re-roll it I can just update the message. If it were >> properly stored in my local history that would also mean I could see >> revisions on it. >> >> Any thoughts on how to do this? >> > > A bit of a plug for patman[1] which the u-boot project uses (although > there's nothing u-boot specific about it). It lets you put the cover > letter and other meta information in the commit messages as you go > then will extract that information and generate a cover letter and > clean patches. As of fairly recently it's also installable as a > standalone application. > > -- > [1] - http://git.denx.de/?p=u-boot.git;a=blob;f=tools/patman/README If you do end up trying it out I'd appreciate any feedback you have. I've sent 1000s of patches through it over the past few years. Regards, Simon -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Junio C Hamano writes: > Ideally, I would think that you want that information when the > series is fully cooked and gets merged to a more permanent place in > the log message of the merge commit. At that point, where the > series started may become more clear from the topology (i.e. the set > difference X^..X for the resulting merge is what got merged). One > possible "hacky" convention could be > > - Developers keep rerolling with the "empty commit with cover >letter material at the tip". topic@{upstream}..topic~1 are the >real changes, topic~0 is an empty "cover letter material". > > - When the series is fully cooked, a new "git merge" option notices >that the topic is structured in a "strange" way, uncaps its tip >commit and merges the remainder of the series and adds the cover >letter material when presenting the editor to record the merge >commit. That is > > $ git merge --cover-at-tip topic > >would work roughly by doing the following: > > - verify that "git rev-parse topic^^{tree} topic^{tree}" shows that > they record the same tree; otherwise it will error out, saying > the tip is not a pure cover. > > - verify that "git rev-list ..topic^" shows that there is > something to merge after the tip is removed; otherwise it will > error out, saying that there is nothing to merge. > > - run "git merge --no-ff --edit topic^1" but with the log > message of topic^{commit} in the editor's template. To complement the above, if we want to pursue this approach, the following would also help. - (obvious) "git pull" would learn the same "--cover-at-tip" option and would pass it to "git merge". - "git am --cover-at-tip" would make the incoming cover-letter material into a non-tree-changing commit at the tip of the resulting topic. - "git format-patch" would notice a topic branch in such a shape and would use the log message of the non-tree-changing commit at the tip as part of the cover letter. Then the overall workflow would become: * A developer works on a topic and concludes it by writing a summary that should appear in the final merge to the trunk as a log message of a non-tree-changing commit at the tip. * In "request-to-pull" workflow, the developer requests the topic to be pulled. The integrator uses "git pull --cover-at-tip" and the resulting merge commit will carry the summary written by the original developer. * In e-mail workflow, the developer runs "git format-patch"; the cover-letter is populated with the summary the developer wrote. * The integrator uses "git am --cover-at-tip" on a new branch, which recreates the topic branch the developer created at the first step above. * The integrator merges the topic with "git merge --cover-at-tip" to the trunk, and the resulting merge commit will carry the summary written by the original developer. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Fri, Sep 11, 2015 at 4:28 AM, Jacob Keller wrote: > Hey, > > does anyone know of any tricks for storing a cover letter for a patch > series inside of git somehow? I'd guess the only obvious way currently > is to store it at the top of the series as an empty commit.. but this > doesn't get emailed *as* the cover letter... > > Is there some other way? Would others be interested in such a feature? > > I get very annoyed when I've written a nice long patch cover letter in > vim before an email and then realize I should fix something else up, > or accidentally cancel it because I didn't use the write "To:" address > or something.. > > I really think it should be possible to store something somehow as a > blob that could be looked up later. Even if this was a slightly more > manual process that would be helpful to store the message inside git > itself. > > In addition, this would help re-rolls since it would mean if I go back > to a topic and re-roll it I can just update the message. If it were > properly stored in my local history that would also mean I could see > revisions on it. > > Any thoughts on how to do this? > A bit of a plug for patman[1] which the u-boot project uses (although there's nothing u-boot specific about it). It lets you put the cover letter and other meta information in the commit messages as you go then will extract that information and generate a cover letter and clean patches. As of fairly recently it's also installable as a standalone application. -- [1] - http://git.denx.de/?p=u-boot.git;a=blob;f=tools/patman/README -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
From: "Johannes Schindelin" On 2015-09-10 23:00, Jacob Keller wrote: On Thu, Sep 10, 2015 at 11:58 AM, Johannes Schindelin wrote: On 2015-09-10 18:28, Jacob Keller wrote: does anyone know of any tricks for storing a cover letter for a patch series inside of git somehow? It is not stored as a blob, but I use `git branch --edit-description` to write the cover letter for patch series when I expect a couple of iterations. Does this (or can it?) get used by send-email or format-patch's --cover-letter? This sounds like exactly what I want. Yes, format-patch picks it up if you say `--cover-letter`. I didn't know that. It doesn't appear to be mentioned in the man pages. IIUC https://github.com/git/git/blob/master/builtin/log.c#L971 suggests that it is a deliberate extra inclusion, rather than being part of the shortlog and diffstat mentioned in the manual https://github.com/git/git/blob/master/Documentation/git-format-patch.txt#L216 Sounds like it may be a worthwhile doc patch. -- Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Hi Jake, On 2015-09-10 23:00, Jacob Keller wrote: > On Thu, Sep 10, 2015 at 11:58 AM, Johannes Schindelin > wrote: >> >> On 2015-09-10 18:28, Jacob Keller wrote: >> >>> does anyone know of any tricks for storing a cover letter for a patch >>> series inside of git somehow? >> >> It is not stored as a blob, but I use `git branch --edit-description` to >> write the cover letter for patch series when I expect a couple of iterations. > > Does this (or can it?) get used by send-email or format-patch's > --cover-letter? This sounds like exactly what I want. Yes, format-patch picks it up if you say `--cover-letter`. Ciao, Johannes P.S.: Please do cut down the quoted text to the part you are actually responding to. Bottom-posting is not much better than top-posting... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Thu, Sep 10, 2015 at 1:09 PM, Philip Oakley wrote: > From: "Jacob Keller" >> >> On Thu, Sep 10, 2015 at 11:44 AM, Junio C Hamano >> wrote: >>> >>> Jacob Keller writes: >>> I hadn't thought of separating the cover letter from git-send-email. That would be suitable for me. >>> >>> >>> Yeah, I said this number of times over time, and I said it once >>> recently in another thread, but I think it was a mistake to allow >>> git-send-email to drive format-patch. It may appear that it will >>> make things convenient in the perfect world where no user makes >>> mistakes, but people are not perfect in real life. Expecting them >>> to be is being naive. >>> >> >> Yep. I didn't even know cover-letter was an option of format-patch >> only thought it was in send-email. >> > Actually, the one feature I'd like (I think) is to be able to join together > the empty commit mechanism and the cover letter mechanism within format > patch so that: > > * the empty commit message would detected and automatically become the [0/N] > in the patch series (without need to say --cover-letter) > > * the cover letter would still have some 'template' markings to say "*** > insert what's changed here***" or smilar (with option to exclude them). > > That way, when starting a series / branch, the first item would be to add > the explanatory 'empty commit' that states the requirements of what one > hopes to achieve (a key cover letter content), which is then followed by > commits that move toward that goal. > > The series can then be rebased as the user develops the code, and that cover > note can be edited as required during the rebase. > > When it comes time to show it to the list, the format patch will *know* from > the empty commit that it is the [0/N] cover letter and (perhaps -option) add > the appropriate markers ready for editing. > > The user edits the cover letter with the extra 'what's changed' / interdiff > / whatever, and sends. sendmail barfs if the user hasn't edited the markers. > > This could also work with the sendmail patch formating (though I've never > used that workflow) as now the cover letter becomes automatic for the > upstream. > > Philip If there was a way to store this empty commit message tagged as "cover letter" that could work well, though generally I prefer the non-fast-forward merges as this shows you where the series ended *and* began. It's somewhat confusing to newer users.. and this doesn't get rebased very well either. Some way to indicate a particular "empty" commit is actually a cover letter seems easy enough. This seems like the way that I was thinking. Using "edit description" of git-branch seems also to be pretty effective for this, even if it doesn't get shared across remotes. (not really a necessary feature for what I do). But having some way to indicate "cover letter" which gets used as the beginning of a log message when doing a particular "merge --tip-as-cover" or something like Junio suggested above seems like the nicest approach. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Thu, Sep 10, 2015 at 11:58 AM, Johannes Schindelin wrote: > Hi Jake, > > On 2015-09-10 18:28, Jacob Keller wrote: > >> does anyone know of any tricks for storing a cover letter for a patch >> series inside of git somehow? > > It is not stored as a blob, but I use `git branch --edit-description` to > write the cover letter for patch series when I expect a couple of iterations. > > Ciao, > Dscho Does this (or can it?) get used by send-email or format-patch's --cover-letter? This sounds like exactly what I want. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
From: "Jacob Keller" On Thu, Sep 10, 2015 at 11:44 AM, Junio C Hamano wrote: Jacob Keller writes: I hadn't thought of separating the cover letter from git-send-email. That would be suitable for me. Yeah, I said this number of times over time, and I said it once recently in another thread, but I think it was a mistake to allow git-send-email to drive format-patch. It may appear that it will make things convenient in the perfect world where no user makes mistakes, but people are not perfect in real life. Expecting them to be is being naive. Yep. I didn't even know cover-letter was an option of format-patch only thought it was in send-email. Actually, the one feature I'd like (I think) is to be able to join together the empty commit mechanism and the cover letter mechanism within format patch so that: * the empty commit message would detected and automatically become the [0/N] in the patch series (without need to say --cover-letter) * the cover letter would still have some 'template' markings to say "*** insert what's changed here***" or smilar (with option to exclude them). That way, when starting a series / branch, the first item would be to add the explanatory 'empty commit' that states the requirements of what one hopes to achieve (a key cover letter content), which is then followed by commits that move toward that goal. The series can then be rebased as the user develops the code, and that cover note can be edited as required during the rebase. When it comes time to show it to the list, the format patch will *know* from the empty commit that it is the [0/N] cover letter and (perhaps -option) add the appropriate markers ready for editing. The user edits the cover letter with the extra 'what's changed' / interdiff / whatever, and sends. sendmail barfs if the user hasn't edited the markers. This could also work with the sendmail patch formating (though I've never used that workflow) as now the cover letter becomes automatic for the upstream. Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Hi Jake, On 2015-09-10 18:28, Jacob Keller wrote: > does anyone know of any tricks for storing a cover letter for a patch > series inside of git somehow? It is not stored as a blob, but I use `git branch --edit-description` to write the cover letter for patch series when I expect a couple of iterations. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Thu, Sep 10, 2015 at 11:44 AM, Junio C Hamano wrote: > Jacob Keller writes: > >> I hadn't thought of separating the cover letter from git-send-email. >> That would be suitable for me. > > Yeah, I said this number of times over time, and I said it once > recently in another thread, but I think it was a mistake to allow > git-send-email to drive format-patch. It may appear that it will > make things convenient in the perfect world where no user makes > mistakes, but people are not perfect in real life. Expecting them > to be is being naive. > Yep. I didn't even know cover-letter was an option of format-patch only thought it was in send-email. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Jacob Keller writes: > I hadn't thought of separating the cover letter from git-send-email. > That would be suitable for me. Yeah, I said this number of times over time, and I said it once recently in another thread, but I think it was a mistake to allow git-send-email to drive format-patch. It may appear that it will make things convenient in the perfect world where no user makes mistakes, but people are not perfect in real life. Expecting them to be is being naive. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Martin Fick writes: > As a Gerrit developer and user, I would like a way to > see/review cover letters in Gerrit. We have had many > internal proposals, most based on git notes, but we have > also used the empty commit trick. It would be nice if there > were some standard git way to do this so that Gerrit and > other tools could benefit from this standard. I am not > suggesting that git need to be modified to do this, but > rather that at least some convention be established. Some of what you would write in the cover letter is not meant for anywhere in the permanent history (e.g. description of what changed since the previous reroll), but some other would be a concise summary of what the entire series is about, and it would be nice if it can be made part of the permanent record. The problem with "empty commit trick" is that it is a commit whose sole purpose is to describe the series, and its presence makes it clear where the series ends, but the topology does not tell where the series begins, so it is an unsatisifactory half-measure. Ideally, I would think that you want that information when the series is fully cooked and gets merged to a more permanent place in the log message of the merge commit. At that point, where the series started may become more clear from the topology (i.e. the set difference X^..X for the resulting merge is what got merged). One possible "hacky" convention could be - Developers keep rerolling with the "empty commit with cover letter material at the tip". topic@{upstream}..topic~1 are the real changes, topic~0 is an empty "cover letter material". - When the series is fully cooked, a new "git merge" option notices that the topic is structured in a "strange" way, uncaps its tip commit and merges the remainder of the series and adds the cover letter material when presenting the editor to record the merge commit. That is $ git merge --cover-at-tip topic would work roughly by doing the following: - verify that "git rev-parse topic^^{tree} topic^{tree}" shows that they record the same tree; otherwise it will error out, saying the tip is not a pure cover. - verify that "git rev-list ..topic^" shows that there is something to merge after the tip is removed; otherwise it will error out, saying that there is nothing to merge. - run "git merge --no-ff --edit topic^1" but with the log message of topic^{commit} in the editor's template. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Thu, Sep 10, 2015 at 11:02 AM, Martin Fick wrote: > On Thursday, September 10, 2015 10:41:54 AM Junio C Hamano > wrote: >> >> I think "should" is too strong here. Yes, you could >> implement that way. It is debatable if it is better, or >> a flat file kept in a directory (my-topic/ in the example >> above) across rerolls is more flexible, lightweight and >> with less mental burden to the users. -- > > As a Gerrit developer and user, I would like a way to > see/review cover letters in Gerrit. We have had many > internal proposals, most based on git notes, but we have > also used the empty commit trick. It would be nice if there > were some standard git way to do this so that Gerrit and > other tools could benefit from this standard. I am not > suggesting that git need to be modified to do this, but > rather that at least some convention be established. > > -Martin > Having used gerrit, this would be useful as well. The "empty commit message" thing sort of works, but has issues. I don't know if this could be solved for gerrit at all without modification to git, since you'd need something that can be sent to the gerrit server and received by the client. Some form of git-notes might work, ie: a git-notes on the first commit, stored in some "standard" refs/notes/cover or similar.. but this would depend on implementation of a standard way to share notes. One alternative as well is to use a --no-ff merge commit which forces the merge between the base and the tip of the series and contains the contents.. but I don't believe gerrit really works well with merge commits. But again, Junio's solution will work great for emails workflow, which is my primary usage. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
On Thu, Sep 10, 2015 at 10:41 AM, Junio C Hamano wrote: > Jacob Keller writes: > >> Is there some other way? Would others be interested in such a feature? > > Not me. > >> I get very annoyed when I've written a nice long patch cover letter in >> vim before an email and then realize I should fix something else up, >> or accidentally cancel it because I didn't use the write "To:" address >> or something.. > > I smell a fallout of encouraging a suboptimal workflow made by > git-send-email here. If you did not know that the command can drive > format-patch itself, your workflow would have been: > > $ git format-patch -o my-topic --cover master..my-topic > $ vi my-topic/*.txt > > and only after you gain confidence with the edited result > > $ git send-email $args my-topic/*.txt > I hadn't thought of separating the cover letter from git-send-email. That would be suitable for me. > which has no room for your grief/complaint to come into the > picture. While rerolling, you can do the same > > $ git format-patch -o my-topic --cover -v2 master..my-topic > > and reuse major parts of cover letter from the original round. > >> I really think it should be possible to store something somehow as a >> blob that could be looked up later. > > I think "should" is too strong here. Yes, you could implement that > way. It is debatable if it is better, or a flat file kept in a > directory (my-topic/ in the example above) across rerolls is more > flexible, lightweight and with less mental burden to the users. This seems reasonable from an email point of view. Thanks for the insight. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
+repo-disc...@googlegroups.com (to hit Gerrit developers also) On Thursday, September 10, 2015 09:28:52 AM Jacob Keller wrote: > does anyone know of any tricks for storing a cover letter > for a patch series inside of git somehow? I'd guess the > only obvious way currently is to store it at the top of > the series as an empty commit.. but this doesn't get > emailed as the cover letter... ... > I really think it should be possible to store something > somehow as a blob that could be looked up later. On Thursday, September 10, 2015 10:41:54 AM Junio C Hamano wrote: > > I think "should" is too strong here. Yes, you could > implement that way. It is debatable if it is better, or > a flat file kept in a directory (my-topic/ in the example > above) across rerolls is more flexible, lightweight and > with less mental burden to the users. -- As a Gerrit developer and user, I would like a way to see/review cover letters in Gerrit. We have had many internal proposals, most based on git notes, but we have also used the empty commit trick. It would be nice if there were some standard git way to do this so that Gerrit and other tools could benefit from this standard. I am not suggesting that git need to be modified to do this, but rather that at least some convention be established. -Martin -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: storing cover letter of a patch series?
Jacob Keller writes: > Is there some other way? Would others be interested in such a feature? Not me. > I get very annoyed when I've written a nice long patch cover letter in > vim before an email and then realize I should fix something else up, > or accidentally cancel it because I didn't use the write "To:" address > or something.. I smell a fallout of encouraging a suboptimal workflow made by git-send-email here. If you did not know that the command can drive format-patch itself, your workflow would have been: $ git format-patch -o my-topic --cover master..my-topic $ vi my-topic/*.txt and only after you gain confidence with the edited result $ git send-email $args my-topic/*.txt which has no room for your grief/complaint to come into the picture. While rerolling, you can do the same $ git format-patch -o my-topic --cover -v2 master..my-topic and reuse major parts of cover letter from the original round. > I really think it should be possible to store something somehow as a > blob that could be looked up later. I think "should" is too strong here. Yes, you could implement that way. It is debatable if it is better, or a flat file kept in a directory (my-topic/ in the example above) across rerolls is more flexible, lightweight and with less mental burden to the users. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html