| Message ID | 20210228195414.21372-5-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | describe: dont abort too early when searching tags | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > @@ -220,6 +225,9 @@ test_commit () { > --no-tag) > no_tag=yes > ;; > + --annotated-tag) > + annotated_tag=yes > + ;; A new option is welcome, but shouldn't we be straightening out the variables that keep track of the options around tagging? It's not like it is possible to have 4 (two variables that can independently set to 'yes') possibilities, so something along the lines of ... test_commit () { + tag=light && notick= && ... while test $# != 0 do case "$1" in ... --no-tag) - no_tag=yes + tag=none ;; + --annotated) + tag=annotated + ;; ... - if test -z "$no_tag" - then + case "$tag" in + none) + ;; + light) git ${indir:+ -C "$indir") tag "${4:-$1}" + ;; + annotated) + git ${indir:+ -C "$indir") tag -m "$1" "${4:-$1}" + ;; + esac ... after this step (meaning, we may want to start from fixing the no_tag=yes to tag=none before introducing this new feature). Thanks. > @@ -244,7 +252,15 @@ test_commit () { > $signoff -m "$1" && > if test -z "$no_tag" > then > - git ${indir:+ -C "$indir"} tag "${4:-$1}" > + if test -n "$annotated_tag" > + then > + if test -z "$notick" > + then > + test_tick > + fi && > + test_tick > + fi && > + git ${indir:+ -C "$indir"} tag ${annotated_tag:+ -a -m "$1"} "${4:-$1}" > fi > }
On Mon, Mar 01 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> @@ -220,6 +225,9 @@ test_commit () { >> --no-tag) >> no_tag=yes >> ;; >> + --annotated-tag) >> + annotated_tag=yes >> + ;; > > A new option is welcome, but shouldn't we be straightening out the > variables that keep track of the options around tagging? It's not > like it is possible to have 4 (two variables that can independently > set to 'yes') possibilities, so something along the lines of ... > > test_commit () { > + tag=light && > notick= && > ... > while test $# != 0 > do > case "$1" in > ... > --no-tag) > - no_tag=yes > + tag=none > ;; > + --annotated) > + tag=annotated > + ;; > ... > - if test -z "$no_tag" > - then > + case "$tag" in > + none) > + ;; > + light) > git ${indir:+ -C "$indir") tag "${4:-$1}" > + ;; > + annotated) > + git ${indir:+ -C "$indir") tag -m "$1" "${4:-$1}" > + ;; > + esac > ... > > after this step (meaning, we may want to start from fixing the no_tag=yes > to tag=none before introducing this new feature). Yeah, as noted in the last paragraph of the commit message: The placement of --annotated-tag after "notick" in the case of the documentation, and then after "no_tag" in the case of the code is slightly inconsistent. It's to evade a merge conflict with two other commits adding a --printf option, and another one adding documentation for --no-tag. So the existing patch + not doing a bigger refactoring is because I didn't want to cause conflicts for you to solve with other in-flight topics. It would be easier with ab/pickaxe-pcre2 merged down :) I'd prefer to keep this as-is for now, and just refactor this function a bit after the current topics land. In particular I'd like to make the the "message file contents tags" arguments optionally have --long-option versions, so e.g. tests that need a separate --commit-message and --tag-message can use the helper, there's also quite a few that could use a --file=<existing file> v.s. echo-ing a "message" to a "file". >> @@ -244,7 +252,15 @@ test_commit () { >> $signoff -m "$1" && >> if test -z "$no_tag" >> then >> - git ${indir:+ -C "$indir"} tag "${4:-$1}" >> + if test -n "$annotated_tag" >> + then >> + if test -z "$notick" >> + then >> + test_tick >> + fi && >> + test_tick >> + fi && >> + git ${indir:+ -C "$indir"} tag ${annotated_tag:+ -a -m "$1"} "${4:-$1}" >> fi >> }
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Yeah, as noted in the last paragraph of the commit message: > > The placement of --annotated-tag after "notick" in the case of the > documentation, and then after "no_tag" in the case of the code is > slightly inconsistent. It's to evade a merge conflict with two other > commits adding a --printf option, and another one adding documentation > for --no-tag. > > So the existing patch + not doing a bigger refactoring is because I > didn't want to cause conflicts for you to solve with other in-flight > topics. It would be easier with ab/pickaxe-pcre2 merged down :) As things that are not yet in 'master' will not be merged down for a few weeks anyway, the conflicts don't matter that much---it is a valid choice not to accept new topics that conflicts with topics cooking in 'next' after all. That way, the number of topics that folks have to look at will not increase, which gives more opportunity for topics that are already in 'seen' to get reviewed ;-)
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh index 6ce62f878c..7c873033e9 100755 --- a/t/t1403-show-ref.sh +++ b/t/t1403-show-ref.sh @@ -7,11 +7,9 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh test_expect_success setup ' - test_commit A && - git tag -f -a -m "annotated A" A && + test_commit --annotated-tag A && git checkout -b side && - test_commit B && - git tag -f -a -m "annotated B" B && + test_commit --annotated-tag B && git checkout main && test_commit C && git branch B A^0 diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 6348e8d733..c6cdabf53e 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -171,6 +171,10 @@ debug () { # Run all git commands in directory <dir> # --notick # Do not call test_tick before making a commit +# --annotated-tag +# Create an annotated tag with "-a -m <message>". Calls +# test_tick between making the commit and tag unless --notick is +# given. # --append # Use "echo >>" instead of "echo >" when writing "<contents>" to # "<file>" @@ -191,6 +195,7 @@ test_commit () { signoff= && indir= && no_tag= && + annotated_tag= && while test $# != 0 do case "$1" in @@ -220,6 +225,9 @@ test_commit () { --no-tag) no_tag=yes ;; + --annotated-tag) + annotated_tag=yes + ;; *) break ;; @@ -244,7 +252,15 @@ test_commit () { $signoff -m "$1" && if test -z "$no_tag" then - git ${indir:+ -C "$indir"} tag "${4:-$1}" + if test -n "$annotated_tag" + then + if test -z "$notick" + then + test_tick + fi && + test_tick + fi && + git ${indir:+ -C "$indir"} tag ${annotated_tag:+ -a -m "$1"} "${4:-$1}" fi }
Add an --annotated-tag option to test_commit. The tag will share the same message as the commit, and we'll call test_tick before creating it (unless --notick) is provided. There's quite a few tests that could be simplified with this construct. I've picked one to convert in this change as a demonstration. The placement of --annotated-tag after "notick" in the case of the documentation, and then after "no_tag" in the case of the code is slightly inconsistent. It's to evade a merge conflict with two other commits adding a --printf option, and another one adding documentation for --no-tag. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t1403-show-ref.sh | 6 ++---- t/test-lib-functions.sh | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 5 deletions(-)