Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

discussion about git pr push -r -n and defaults #47

Open
bast opened this issue Sep 3, 2020 · 14 comments
Open

discussion about git pr push -r -n and defaults #47

bast opened this issue Sep 3, 2020 · 14 comments

Comments

@bast
Copy link
Member

bast commented Sep 3, 2020

How do you typically use git pr push? Are you using -r -n always or only sometimes?

I think the default should be what we do most of the time but unsure what we do most of the time? I can imagine myself typing -r -n always.

These are not too easy to remember. What is -r short for?

@rkdarst
Copy link
Member

rkdarst commented Sep 3, 2020 via email

@bast
Copy link
Member Author

bast commented Sep 3, 2020

For me I anticipate that I will use git pr for the "smaller" PRs where I don't want an extra step on the web. For elaborate PRs where I need to very carefully prepare a PR description based on a number of commits, with cross-references to issues etc, I might go via web.

This means that I would probably prefer git pr push to always open it and rather have a flag that does not open it. But git pr push perhaps does not convey that it does more than pushing. I was thinking about git pr open? But does that convey to the user that something is pushed?

Of course I can solve this with an alias but wanted to discuss good defaults before this tool goes viral and it becomes harder to change defaults.

@bast
Copy link
Member Author

bast commented Sep 3, 2020

The more I think about it the more I like git pr open. It opens a PR. Does not matter how it does it, by pushing and stuff. All I want is that it opens the PR for me and I can move on.

@bast
Copy link
Member Author

bast commented Sep 3, 2020

But if git pr open exists, can it be confused with git pr new? ("new pull request?")

If I did not know what git branch does I would consider renaming git pr new to git pr branch.

@rkdarst
Copy link
Member

rkdarst commented Sep 3, 2020 via email

@bast
Copy link
Member Author

bast commented Sep 3, 2020

The new checkout -b is switch --create so we could also consider git pr create but that again might sound like creating a pull request. git pr switch? git pr switch --create is too long.

@rkdarst
Copy link
Member

rkdarst commented Sep 3, 2020 via email

@bast
Copy link
Member Author

bast commented Sep 3, 2020

Sounds good:

  • git pr branch instead of git pr new
  • keep git pr push which does not do -r by default (still I find -r non-intuitive character)
  • git pr open is porcelain for git pr -r (how about -n?)

@rkdarst
Copy link
Member

rkdarst commented Sep 3, 2020

The -n came from --no-edit in the hub Github API interface, and
seems to be an invention of mine.

What about push -o for "push and open", to be consistent with the new open? (-n also works, though) ? (-r was "Request it in addition to push").

Here is the reference for hub:
https://hub.github.com/hub-pull-request.1.html
And should also avoid clashing with the new cli:
https://cli.github.com/manual/gh_pr_create
Neither -o nor -n are taken in either, so it's OK to use either. -r is already taken.

@bast
Copy link
Member Author

bast commented Sep 3, 2020

I like git pr push -o instead of git pr push -r.

And -n is intuitive but should it not be default and we rather have git pr open -e|--edit to open the editor once more?

@bast
Copy link
Member Author

bast commented Sep 3, 2020

And although commands can reference each other and backwards compatibility is "no problem", I would still limit the option to not arrive at something like Git where there are some options where I ask myself whether Linus has used them twice and now we are stuck with them.

So perhaps instead:

$ git pr push mybranch
$ git pr push -o mybranch

we could do:

$ git pr push mybranch  # here the user preferred to not open yet
$ git pr open mybranch

@rkdarst
Copy link
Member

rkdarst commented Sep 3, 2020

I still sort of like the principle of least surprise: it's safe to open the editor (pre-filled with the message it would have). But not doing that could result in spam to projects when someone doesn't realize it will immediately open it and mail everyone.

we could do:
$ git pr push mybranch
$ git pr open mybranch

In fact that's sort of how this evolved: there was git pr push (push) and git pr gh (to open the pull request itself on Github). I wanted even fewer keystrokes, so added -r, then stopped using gh I think keeping the things separate in the "base case" is a good idea.

To make it worse, Gitlab has different semantics:
https://docs.gitlab.com/ee/user/project/push_options.html
The way you open it is git push --push-option=merge_request.create, so it has to be a part of push somehow.

Should we have a video chat to talk about it? I feel this is going on quite a while, and more quick discussions=better.

rkdarst added a commit that referenced this issue Sep 3, 2020
- "new" was too similar to the concept of creating a new pull request,
  so it has been renamed to "branch".  "new" still serves as an
  undocumented alias.
- To review: Not much, should be straightforward after the discussion
  in #47.
- Part of solving #47.
@rkdarst
Copy link
Member

rkdarst commented Sep 3, 2020

The branch one seems simple so I did that one first.

@bast
Copy link
Member Author

bast commented Sep 3, 2020

Good convincing argument to not make -n default. Yes we can talk over video if there is something to discuss about the GitLab part. And I highly recommend to limit the number of options and aliases and not to keep any undocumented ones. I expect that this will hurt in future. We can talk about it over video.

rkdarst added a commit that referenced this issue Sep 4, 2020
- open command: This is an alias for `git pr -o`, but provides a more
  logical entry point to recommend to people when they are starting.
  Open always pushes at the same time.
- This concept may still be developed, this is only the initial
  change.
- Remove documentation of the `gh` subcommand, it is almost never used
  directly.
- Part of #47.
rkdarst added a commit that referenced this issue Sep 4, 2020
- open command: This is an alias for `git pr -o`, but provides a more
  logical entry point to recommend to people when they are starting.
  Open always pushes at the same time.
- This concept may still be developed, this is only the initial
  change.
- Remove documentation of the `gh` subcommand, it is almost never used
  directly.
- Part of #47.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants