forge icon indicating copy to clipboard operation
forge copied to clipboard

Add support to assign reviewer / approver

Open dcaixinha opened this issue 4 years ago • 13 comments

Hey @tarsius, first of all thanks for this incredibly amazing package :tada:

One thing that I'd love to have is the ability to assign the reviewers for a Pull Request from forge. At work I'm using GitLab and after some digging it looks like they have an endpoint to do this.

I'm an Emacs Lisp noob but would be up to study this package and try to make a contribution. Would you be open to such a contribution? In your opinion should this functionality work for GitLab and GitHub at the same time? If you're open for such contribution, please give me some pointers on where to being :pray:

Thanks again :bowing_man:

dcaixinha avatar Feb 11 '20 17:02 dcaixinha

This already supported. Move the cursor onto Assignees: none and press C-c C-e.

tarsius avatar Feb 17 '20 14:02 tarsius

Is it possible to assign reviewers when initially creating a pull request?

I'm a heavy Magit user and and Forge looks perfect for working with GitHub. Previously I used https://github.com/github/hub where I could create a pull request and assign reviewer(s) in a single step with hub pull-request -r username.

mpolden avatar Feb 19 '20 17:02 mpolden

That's not supported yet. If you want to implement this yourself, then look at forge--submit-create-issue and forge--topic-parse-yaml, and draw your conclusions from that.

tarsius avatar Feb 19 '20 19:02 tarsius

@tarsius but your suggestion (to use C-c C-e) will edit/add an Assignee to the MR, not a reviewer, right? My question was to have this exact same functionality but for reviewers.

dcaixinha avatar Feb 20 '20 11:02 dcaixinha

Yes, but my advice "look at how it is done for X and do the same for Y" still stands.

tarsius avatar Feb 23 '20 15:02 tarsius

Looks like I was wrong though.

  1. Forge does not provide an interface for conveniently setting the assignee (or to request a review). It does however support issue templates and the user can manually write what one would put into such a template. But its inconvenient.
  2. Issue templates support assignees but not review requests so we actually can't "just do the same" here.

See https://help.github.com/en/github/building-a-strong-community/configuring-issue-templates-for-your-repository#creating-issue-templates.

tarsius avatar Feb 23 '20 15:02 tarsius

I think we're talking about two different issues here:

  1. The ability to set a reviewer the same way we're able to set an assignee (as we do with C-c C-e on a PR – especially in GitLab (is this already possible with GitHub?)
  2. The ability to create a PR an set a reviewer in one single step

Should this be split into 2 issues?

dcaixinha avatar Feb 28 '20 12:02 dcaixinha

Should this be split into 2 issues?

No.

tarsius avatar Feb 28 '20 13:02 tarsius

Any reason why? Do you agree these are two different issues?

dcaixinha avatar Feb 28 '20 14:02 dcaixinha

While maybe not the same, they are still closely related and I will likely tackle both of them at the same time. Also I sleep better if there are fewer issues.

tarsius avatar Feb 28 '20 14:02 tarsius

This comment might be of interest to people who are researching this subject.

I’ve searched for a way to change the reviewer & assignee for GitLab Merge Requests (MR) from inside forge, because I was always visiting the website, which is kind of clunky and slow compared to an Emacs-native workflow.

So in addition to the ability to use C-c C-e on the corresponding fields in the topic, I discovered that there are already two functions present in the current version of forge that aid in both use cases: forge-edit-topic-assignees and forge-edit-topic-review-requests (I couldn’t find a way to add a field to a forge topic, and I prefer evil-like key bindings, hence the search for an alternative solution).

After I discovered that these two functions exist and indeed work perfectly for my use case, I tried to add them to the forge transient buffer, but initially failed, because the available documentation is of a rather theoretical nature, and I’m not proficient enough with either Lisp or the Emacs ecosystem to build a working example out of it. After some web research I finally cobbled together a working solution, added it to my init.el, and it looks way easier than initially thought:

(require 'forge)
(transient-append-suffix 'forge-dispatch '(0)
 ["Edit"
  ("e a" "assignees" forge-edit-topic-assignees)
  ("e r" "review requests" forge-edit-topic-review-requests)])

I didn’t meant to hijack this thread, but in my opinion the above addition already solves this issue for me.

rbugajewski avatar Apr 15 '21 12:04 rbugajewski

For anyone finding this in 2023 and onward, you can assign a reviewer after a pull request is open by selecting the Review-Requests line and hitting C-c C-e

Screenshot 2023-03-22 at 4 35 25 PM

gf3 avatar Mar 22 '23 20:03 gf3

any way to assign a team under an organization with this mechanism (e.g. org/team)? all I see are users.

bklebe avatar Oct 31 '23 19:10 bklebe