vermiculus / magithub

**DEPRECATED - please use Forge instead!** -- Magit-based interfaces to GitHub
GNU General Public License v3.0
579 stars 63 forks source link

PR workflow improvement ideas #264

Open aaronjensen opened 6 years ago

aaronjensen commented 6 years ago

As I mentioned in my other PR, I've been using magit-gh-pulls to create PRs. Its workflow is pretty slim and as such, very fast and nice. Magithub appears to take the tact of asking for confirmation for everything I do. It'd be great if I could set a var skip confirmations.

The current flow is something like:

  1. @ p
  2. Ask for remote (this is fine, but it'd be great if the choice were remembered).
  3. Ask for Head branch (this is a confusing name to me, maybe "Your branch" may be less confusing? It defaults to the current branch which is good.
  4. Base branch (this defaults to HEAD for me, which allows me to go through attempting to create a PR but it then tells me the PR already exists. It'd be great if it defaulted to the default branch on GH or if it defaulted to the last thing I chose, or if it were configurable)
  5. Ask if I'm sure? (please allow this to be skipped, I can always cancel on the PR message entry if things look wrong)
  6. Edit message
  7. C-c C-c
  8. Ask if I'm sure? (please allow this to be skipped, I don't think I've ever accidentally hit C-c C-c)
  9. Ask if I want to allow owner modifications? (this is cool, though I'd prefer if I could set a default for this and maybe toggle it while on the PR message rather than being asked).

I hope that's not too many requests all in one issue πŸ˜„

Thanks!

vermiculus commented 6 years ago

Thanks for the feedback!

(4) sounds like a bug. In this workflow, can you evaluate (magit-get-upstream-branch (magit-get-current-branch))? If it returns "HEAD", that's why the default is what it is, though I don't see how that makes sense. Otherwise, can you give a more specific workflow to reproduce? (That can probably go into a new issue for tracking.)

(5, 8) This will be addressed πŸ˜„ I've got a new branch to do just this (sa/confirm). You'll be able to pre-set answers to these (and many other) questions using git config variables. Take a peek at magithub-settings.el:magithub-confirmation if you're interested.

(9) This is sorta being addressed with sa/confirm, but perhaps not completely. Can you expand on what you mean by toggling it while on the PR message? (Do you mean like some piece of the PR that is not actually part of the title or body? That may be difficulty to do, technically speaking.)

aaronjensen commented 6 years ago

5/8, great!

9, I'm not sure there is precedent for it, but I'm somewhat thinking of it like the way that magit handles flags. Before a commit I could hit - h to disable hooks, for example, but I was imagining being able to toggle it even while editing the commit message (with some command--the help text could indicate the current status and how to toggle it where it shows to hit C-c to submit). Of course, just being able to toggle it in the magithub panel after hitting @ similar to a flag in the commit panel would be great too.

To be honest, I'd probably just always have that enabled and never uncheck it, so I personally don't need any of this complexity πŸ˜„

vermiculus commented 6 years ago

(9) would need some enhancements to magithub-edit-mode if we were to allow popups (which is a good idea and something magithub used to be able to do when it used hub – which is well before anything could be written to github).

I thought about adding variables to the popup for all this stuff, but it would get out of hand very quickly. How important do you think this is? I might be able to define a popup structure for it reachable from the main one, but it might be funky. Another option would be to implement wrappers around y-or-n-p and yes-or-no-p that could set the default during selection (…I just thought of it, but I like this option… what do you think?).

aaronjensen commented 6 years ago

I think it'd be sufficient for now to make it so that a person can say "always allow", "never allow" or "ask me every time". I'd probably select "always allow" and never look back.

vermiculus commented 6 years ago

But would you want that selection done in emacs or are you (and others in general) alright with running git config --global yourself?

(By the way, these are the options I was planning.)

aaronjensen commented 6 years ago

Well, it'd be great if they could be done in emacs, it's annoying to have to look up magit-gh-pulls's one option every time I need it. If finding the docs for them is intuitive somehow that'd be good too. That said, I'll probably only set these once, so I'm not too worried about it. Maybe someday they can be set from emacs πŸ˜„

Those options look good to me.

vermiculus commented 6 years ago

(2) comes with gotchas for maintainers submitting pull requests for other repositories they have push access to. I'll see if I can make the default always be the remote associated with (magithub-user-me), though (but 'Your branch' may not be accurate, either). Maybe 'Branch with proposed changes'? (It strikes me as kinda long; what do you think?)

(3) Why is 'Head branch' confusing? While I originally got it from the API docs for that parameter, it does make sense to me as 'this is my HEAD, this should be everyone's new HEAD'. I'm open to suggestions, though.

I think that's everything left πŸ˜„

aaronjensen commented 6 years ago

Maybe 2 would be the pushRemote by default, that's where I just pushed to (most likely) and so it's most likely where the PR would come from. For the remembering portion I meant on a per-repository basis, but thinking about it more now, pushRemote makes the most sense--that fits w/ the magit way.

I think it's confusing because they're all heads, HEAD on the target remote is usually master. I think magithub asks for the branches in the opposite order of magit-gh-pulls, which I've been using for a while, so that probably contributed to the confusion. I also liked the clarity of the "this is you!" when picking the remote, it left off the technical words and was straight forward. If no one else is confused by it then i'm not too worried about it.

aaronjensen commented 6 years ago

One other PR workflow issue, if I have more than one commit and a template, the initial message should start with two blank lines so I can type my PR title.