Add reviewers to a PR (WIP)

[bobrowadam]

@vermiculus Any suggestions on where should we get the reviewers list when updating the issue view? I noticed that the reviewers are not part of the issue.

[bobrowadam]

@vermiculus magithub-reviewer-add seems to work. I'm having issues with magithub-reviewer-remove though. When pointing on the reviewers names the value I get for (thing-at-point 'github-user) is nil. Might be related to the following error: Invalid face reference: magit-user Any idea what may cause that?

[vermiculus]

I can look into it locally.

[bobrowadam]

Fetching the collaborators on each refresh is too slow. I will have to think of a better way.

[vermiculus]

Cache them. Use the :issues bucket.

[vermiculus]

Now that I'm finally at a computer again with some time, I want to expand on my previous comment. There's a caching system present in magithub-core.el that exposes function magithub-cache. You can use the definition of magithub-user-me as a guide.

[bobrowadam]

Great, I was planning to implement that tomorrow. Also, let me know if you have any idea why would the reviewers object is 'nil' when applying reviewer-remove.

[vermiculus]

I'd like to experiment with it, but my test repository has no collaborators – I sent you an invite: https://github.com/vermiculus/my-new-repository/invitations

[vermiculus]

I found the problem anyway :-) you're passing the user's ID; not the review ID. Take a peek at the documentation. What will probably need to happen is to search through the open reviews on the PR and find a match on the username – then delete that.

[vermiculus]

Also, if you rebase onto master, that should resolve the issues you're having with CI.

[bobrowadam]

1. Added caching for reviewers, hope I got it right.
2. I was following #delete-a-review-request This API only needs a list of names for deletion. I tested it by using curl and it seems to do the job. Moreover, I would still expect that evaluating (thing-at-point 'github-user) will return the user's list (same as when running it over an assignee object).
3. Unless I'm doing anything wrong, I'm already rebased over master.

[vermiculus]

Some commits have been made since the time your forked – one of those commits fixed the tests that check for clean compile, etc.

[vermiculus]

Looks like there are some compile errors: https://travis-ci.org/vermiculus/magithub/jobs/432728627#L996-L997

magithub-issue.el:422:1:Warning: Unused lexical variable ‘assignee’
magithub-issue.el:429:69:Error: assignment to free variable ‘reviewer’

[bobrowadam]

@vermiculus I think that those errors should be fixed now. The only issue is with ghub's version it seems: https://travis-ci.org/vermiculus/magithub/jobs/444470385#L1024

[vermiculus]

I just merged the vermiculus/ghub-plus#11, so that updated version should be on MELPA shortly :-)

[vermiculus]

There are a few other changes to make (typos and other stuff) but I'll push those, squash, and merge later tonight.

[vermiculus]

(Thanks!!)

[vermiculus]

I think the above issue is due to the user's login/ID being passed as the review ID. These are not the same values.

[vermiculus]

I think I've fixed it – tested that I could add+remove you from vermiculus/my-new-repository#56. Can you test as well to make sure I didn't break something that was working for you before?

[bobrowadam]

@vermiculus when running magithub-reviewer-remove the user value is nil therefore I get "Missing required arguments". Other than that all looks great :)

[bobrowadam]

@vermiculus any updates?

[vermiculus]

I'm sorry I haven't been able to focus on this lately :( hopefully I'll get some time during the thanksgiving weekend

[bobrowadam]

Sure don't worry about it :) I was just wondering if you need anything more from me to do