| Message ID | 20170323210731.38313-2-seanpaul@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 23 Mar 2017, Sean Paul <seanpaul@chromium.org> wrote: > Launch $EDITOR when extracting tags to curate the tags immediately. Once the > tags are proper, automatically add them before the first Signed-off-by line > to all patches in the range. Two problems I have with this. First, I like to edit the final commit message, not an intermediate file. Second, I pipe messages to dim extract-tags *from* my $EDITOR, so I'd rather not have it launch $EDITOR again. BR, Jani. > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > dim | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/dim b/dim > index 43ea794..6a5580a 100755 > --- a/dim > +++ b/dim > @@ -1145,6 +1145,12 @@ function rangeish() > fi > } > > +function insert_extracted_tags > +{ > + new_tabs=`cat $1 | awk '{ORS="\\\\n"} {print $0}' | head -c-3` > + awk "/Signed-off-by/{p++} p==1{print \"$new_tabs\"; p++} p!=1{print}" > +} > + > function dim_extract_tags > { > local branch range file tags > @@ -1167,9 +1173,10 @@ function dim_extract_tags > return 0 > fi > > - tags=$(printf -- "# *** extracted tags ***\n%s" "$tags") > - > - git filter-branch -f --msg-filter "cat ; echo \"$tags\"" $range > + echo "$tags" > $file > + $EDITOR $file > + export -f insert_extracted_tags > + git filter-branch -f --msg-filter "insert_extracted_tags $file" $range > } > > function dim_extract_queued
On Thu, Mar 23, 2017 at 05:07:28PM -0400, Sean Paul wrote: > Launch $EDITOR when extracting tags to curate the tags immediately. Once the > tags are proper, automatically add them before the first Signed-off-by line > to all patches in the range. Not sure adding them before the first sob is the right thing. If we want the tags to reflect the history somehow, then I suppose they should go just before *your* sob, which may or may not be the first sob in the commit message. Personally I've just started slapping them to the end which also maintains the history, but granted it doesn't really show who signed off on those tags. Or is there some "official" rule how tags should be sorted in commit messages? And I don't like the $EDITOR idea either, so at least make it optional. > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > dim | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/dim b/dim > index 43ea794..6a5580a 100755 > --- a/dim > +++ b/dim > @@ -1145,6 +1145,12 @@ function rangeish() > fi > } > > +function insert_extracted_tags > +{ > + new_tabs=`cat $1 | awk '{ORS="\\\\n"} {print $0}' | head -c-3` > + awk "/Signed-off-by/{p++} p==1{print \"$new_tabs\"; p++} p!=1{print}" > +} > + > function dim_extract_tags > { > local branch range file tags > @@ -1167,9 +1173,10 @@ function dim_extract_tags > return 0 > fi > > - tags=$(printf -- "# *** extracted tags ***\n%s" "$tags") > - > - git filter-branch -f --msg-filter "cat ; echo \"$tags\"" $range > + echo "$tags" > $file > + $EDITOR $file > + export -f insert_extracted_tags > + git filter-branch -f --msg-filter "insert_extracted_tags $file" $range > } > > function dim_extract_queued > -- > 2.12.1.500.gab5fba24ee-goog
On Fri, Mar 24, 2017 at 09:54:15AM +0200, Jani Nikula wrote: > On Thu, 23 Mar 2017, Sean Paul <seanpaul@chromium.org> wrote: > > Launch $EDITOR when extracting tags to curate the tags immediately. Once the > > tags are proper, automatically add them before the first Signed-off-by line > > to all patches in the range. > > Two problems I have with this. First, I like to edit the final commit > message, not an intermediate file. Hi Jani, Thanks for your review. The intermediate file includes just the extracted tags, not the entire commit message. This allows you to reorder them as appropriate, or remove any false positives before it hits the log. > Second, I pipe messages to dim > extract-tags *from* my $EDITOR, so I'd rather not have it launch $EDITOR > again. > The idea is that you approve the tags as soon as you extract them and saves you having to commit --amend later to remove the "*** extracted tags ***" lines. This can be meaningful if you've extracted tags to a large set, since you'd need to go through each commit to make any necessary changes. As Ville suggested, I can make the editor step optional (and restore the "*** extracted tags ***" for the disabled case). Sean > BR, > Jani. > > > > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > --- > > dim | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/dim b/dim > > index 43ea794..6a5580a 100755 > > --- a/dim > > +++ b/dim > > @@ -1145,6 +1145,12 @@ function rangeish() > > fi > > } > > > > +function insert_extracted_tags > > +{ > > + new_tabs=`cat $1 | awk '{ORS="\\\\n"} {print $0}' | head -c-3` > > + awk "/Signed-off-by/{p++} p==1{print \"$new_tabs\"; p++} p!=1{print}" > > +} > > + > > function dim_extract_tags > > { > > local branch range file tags > > @@ -1167,9 +1173,10 @@ function dim_extract_tags > > return 0 > > fi > > > > - tags=$(printf -- "# *** extracted tags ***\n%s" "$tags") > > - > > - git filter-branch -f --msg-filter "cat ; echo \"$tags\"" $range > > + echo "$tags" > $file > > + $EDITOR $file > > + export -f insert_extracted_tags > > + git filter-branch -f --msg-filter "insert_extracted_tags $file" $range > > } > > > > function dim_extract_queued > > -- > Jani Nikula, Intel Open Source Technology Center
On Fri, Mar 24, 2017 at 11:07:17AM +0200, Ville Syrjälä wrote: > On Thu, Mar 23, 2017 at 05:07:28PM -0400, Sean Paul wrote: > > Launch $EDITOR when extracting tags to curate the tags immediately. Once the > > tags are proper, automatically add them before the first Signed-off-by line > > to all patches in the range. > > Not sure adding them before the first sob is the right thing. If we want > the tags to reflect the history somehow, then I suppose they should go > just before *your* sob, which may or may not be the first sob in the > commit message. Personally I've just started slapping them to the end > which also maintains the history, but granted it doesn't really show who > signed off on those tags. Or is there some "official" rule how tags > should be sorted in commit messages? Hi Ville, Thanks for your review. I usually sort them based on my perceived strength of the tag and put them before the first SoB. Cc: Reviewed-by: Tested-by: Acked-by: Signed-off-by: I'm fine with putting them above the first SoB of the committer, that makes sense to me. I'm not sure this is something we'll get consensus on, so that seems like a sensible approach. Sean > > And I don't like the $EDITOR idea either, so at least make it > optional. > > > > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > --- > > dim | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/dim b/dim > > index 43ea794..6a5580a 100755 > > --- a/dim > > +++ b/dim > > @@ -1145,6 +1145,12 @@ function rangeish() > > fi > > } > > > > +function insert_extracted_tags > > +{ > > + new_tabs=`cat $1 | awk '{ORS="\\\\n"} {print $0}' | head -c-3` > > + awk "/Signed-off-by/{p++} p==1{print \"$new_tabs\"; p++} p!=1{print}" > > +} > > + > > function dim_extract_tags > > { > > local branch range file tags > > @@ -1167,9 +1173,10 @@ function dim_extract_tags > > return 0 > > fi > > > > - tags=$(printf -- "# *** extracted tags ***\n%s" "$tags") > > - > > - git filter-branch -f --msg-filter "cat ; echo \"$tags\"" $range > > + echo "$tags" > $file > > + $EDITOR $file > > + export -f insert_extracted_tags > > + git filter-branch -f --msg-filter "insert_extracted_tags $file" $range > > } > > > > function dim_extract_queued > > -- > > 2.12.1.500.gab5fba24ee-goog > > -- > Ville Syrjälä > Intel OTC
diff --git a/dim b/dim index 43ea794..6a5580a 100755 --- a/dim +++ b/dim @@ -1145,6 +1145,12 @@ function rangeish() fi } +function insert_extracted_tags +{ + new_tabs=`cat $1 | awk '{ORS="\\\\n"} {print $0}' | head -c-3` + awk "/Signed-off-by/{p++} p==1{print \"$new_tabs\"; p++} p!=1{print}" +} + function dim_extract_tags { local branch range file tags @@ -1167,9 +1173,10 @@ function dim_extract_tags return 0 fi - tags=$(printf -- "# *** extracted tags ***\n%s" "$tags") - - git filter-branch -f --msg-filter "cat ; echo \"$tags\"" $range + echo "$tags" > $file + $EDITOR $file + export -f insert_extracted_tags + git filter-branch -f --msg-filter "insert_extracted_tags $file" $range } function dim_extract_queued
Launch $EDITOR when extracting tags to curate the tags immediately. Once the tags are proper, automatically add them before the first Signed-off-by line to all patches in the range. Signed-off-by: Sean Paul <seanpaul@chromium.org> --- dim | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)