mbox series

[RFC,0/1] push: introduce '--heads' option

Message ID 20221205133525.60464-1-tenglong.tl@alibaba-inc.com (mailing list archive)
Headers show
Series push: introduce '--heads' option | expand

Message

Teng Long Dec. 5, 2022, 1:35 p.m. UTC
From: Teng Long <dyroneteng@gmail.com>

This RFC patch try to introduce a new option '--heads' in 'git-push' subcmd. The
value of this patch may come from my personal point of view, and the patch might
not have enough tests so far. It's pleasure to hear any suggestion, test
scenario which need to be covered or any test method which need to be noticed if
it's worthy.

Thanks.

Teng Long (1):
  push: introduce '--heads' option

 Documentation/git-push.txt |  1 +
 builtin/push.c             | 13 +++++++------
 t/t5523-push-upstream.sh   | 19 ++++++++++++-------
 3 files changed, 20 insertions(+), 13 deletions(-)

Comments

Junio C Hamano Dec. 5, 2022, 11:35 p.m. UTC | #1
Teng Long <dyroneteng@gmail.com> writes:

> From: Teng Long <dyroneteng@gmail.com>
>
> This RFC patch try to introduce a new option '--heads' in 'git-push' subcmd. The
> value of this patch may come from my personal point of view, and the patch might
> not have enough tests so far. It's pleasure to hear any suggestion, test
> scenario which need to be covered or any test method which need to be noticed if
> it's worthy.

My knee-jerk reaction is to avoid "--heads" and instead use
"--branches", if this is about pushing all local branches.  The
option "--heads" may still remain in some commands added to the
system in the earliest part of our history, but soon we started
to use "branch" over "head", as it is a more commonly used word.

How should it interact with --follow-tags?  Just as if you listed
all local branch names on the command line?  I.e. is

    git push $URL --heads

equivalent to the long-hand

    git push $URL $(git for-each-ref --format='%(refname)' refs/heads/\*)

and because of that, does

    git push $URL --any --other --option --heads

behave identically to the long-hand with these other options added?
Teng Long Dec. 15, 2022, 12:27 p.m. UTC | #2
Junio C Hamano <gitster@pobox.com> writes:

> My knee-jerk reaction is to avoid "--heads" and instead use
> "--branches", if this is about pushing all local branches.  The
> option "--heads" may still remain in some commands added to the
> system in the earliest part of our history, but soon we started
> to use "branch" over "head", as it is a more commonly used word.

OK, that's true, I just misled by some existing '--heads' in other
commands.

> How should it interact with --follow-tags?  Just as if you listed
> all local branch names on the command line?  I.e. is

Actually I didn't try '--follow-tags' before, but the documentation
about it is a  ittle hard to understand for me on first reading. Then,
I think it supports to use as negative '--[no-]follow-tags' but not
marked in the git-push.txt documentation.

>     git push $URL --heads
>
> equivalent to the long-hand
>
>     git push $URL $(git for-each-ref --format='%(refname)' refs/heads/\*)

git push $URL $(git for-each-ref --format='%(refname)' refs/heads/\*\*) maybe
to recursivly subdirectories matching?

Actually I didn't get why you represent this, maybe try to let's us know there
is another way we could make it as the same result?

> and because of that, does
>
>     git push $URL --any --other --option --heads
>
> behave identically to the long-hand with these other options added?

I think you concerned about the compatibility with the interaction of
the options, if so, I think a direct way is to keep --all and --heads
both have the some behavior when interact with other options, a little
confused why we have to use the long-hand to do that.

By the way, it seems like there are no specify tests for '--all', maybe
we can add some tests about '--all' first if this RFC patch is worthy to
continue.

Thanks.
Teng Long April 28, 2023, 9:59 a.m. UTC | #3
Junio C Hamano <gitster@pobox.com> writes:

>My knee-jerk reaction is to avoid "--heads" and instead use
>"--branches", if this is about pushing all local branches.  The
>option "--heads" may still remain in some commands added to the
>system in the earliest part of our history, but soon we started
>to use "branch" over "head", as it is a more commonly used word.
>
>How should it interact with --follow-tags?  Just as if you listed
>all local branch names on the command line?  I.e. is
>
>    git push $URL --heads
>
>equivalent to the long-hand
>
>    git push $URL $(git for-each-ref --format='%(refname)' refs/heads/\*)
>
>and because of that, does
>
>    git push $URL --any --other --option --heads
>
>behave identically to the long-hand with these other options added?

ZheNing Hu mentioned me that could use "OPT_ALIAS" instead, it seems
like could be better than OPT_BIT in this scenario. If so, are problems
that may arise from interactions shielded? If not, I'm willing to add
extra test about it (some relevant advice if possible).

Thanks.
Junio C Hamano April 28, 2023, 6:48 p.m. UTC | #4
Teng Long <dyroneteng@gmail.com> writes:

> ZheNing Hu mentioned me that could use "OPT_ALIAS" instead, it seems
> like could be better than OPT_BIT in this scenario. If so, are problems
> that may arise from interactions shielded? If not, I'm willing to add
> extra test about it (some relevant advice if possible).

The intent of ALIAS is to just add an extra option visible at the UI
level that behaves exactly the same as the other one at the code
level, so the codepath that is prepared to deal with one can handle
the other one without any extra effort.  In fact, after the option
parsing is finished, the rest of the code should not even be able to
tell which one, the original or the alias, was used on the command
line.

And in this case, you'd want a new "push all branches" option that
behaves exactly like existing "--all", and possibly you may over
time want to deprecate the latter.  All the code to ensure how
"--all" should interact with other options should be working fine
(or if there is a bug, that needs to be corrected whether we would
add this alias or not).

Sounds like a very good plan to me.

Thanks.
Teng Long April 30, 2023, 1:09 a.m. UTC | #5
Junio C Hamano <gitster@pobox.com> writes:

>Teng Long <dyroneteng@gmail.com> writes:
>
>> ZheNing Hu mentioned me that could use "OPT_ALIAS" instead, it seems
>> like could be better than OPT_BIT in this scenario. If so, are problems
>> that may arise from interactions shielded? If not, I'm willing to add
>> extra test about it (some relevant advice if possible).
>
>The intent of ALIAS is to just add an extra option visible at the UI
>level that behaves exactly the same as the other one at the code
>level, so the codepath that is prepared to deal with one can handle
>the other one without any extra effort.  In fact, after the option
>parsing is finished, the rest of the code should not even be able to
>tell which one, the original or the alias, was used on the command
>line.
>
>And in this case, you'd want a new "push all branches" option that
>behaves exactly like existing "--all", and possibly you may over
>time want to deprecate the latter.  All the code to ensure how
>"--all" should interact with other options should be working fine
>(or if there is a bug, that needs to be corrected whether we would
>add this alias or not).

Make sense.

Thanks.
Teng Long May 6, 2023, 11:34 a.m. UTC | #6
From: Teng Long <dyroneteng@gmail.com>

Based on the feedback I got from the previous RFC
patch, I made a formal patch with some additional
test cases, but I don't know if the scenarios covered by
the test cases are sufficient, if not, the cases will be
improved in a subsequent patch.

Thanks.

Teng Long (1):
  push: introduce '--branches' option

 Documentation/git-push.txt |   3 +-
 builtin/push.c             |   7 ++-
 t/t5523-push-upstream.sh   |  12 +++-
 t/t5543-atomic-push.sh     |   5 +-
 t/t5583-push-branches.sh   | 115 +++++++++++++++++++++++++++++++++++++
 5 files changed, 135 insertions(+), 7 deletions(-)
 create mode 100755 t/t5583-push-branches.sh

Range-diff:
1:  9c9438c3 < -:  -------- push: introduce '--heads' option
-:  -------- > 1:  b16bdfe6 push: introduce '--branches' option