charignon / github-review Goto Github PK

I receive the "Error talking to GitHub" message when I try to submit a review on a repository where I am given "maintainer" rights.

Notice that when I am testing against a repository that I created and I own, everything works fine.

When i try to start an pull request review i got Got an error from the GitHub API (wrong-type-argument listp error)!

Would be nice to have syntax highlight for the reviewed files, getting the major mode with emacs, doing like org do on +src.

It would be nice if github-review used graphql instead of api v3 to reduce the data transferred and the number of network call it performs. The only difficulty is for fetching the "diff" itself as it does not seem supported by the graphql endpoint.

Error talking to GitHub isn't an actionable error message, we should instead surface the error message that GitHub's API is sending back in case of error.

Would be nice to have it in its own mode, instead of diff-mode (from which it can still derive) so there's a mode-map etc.

It would be awesome to have repos/:owner/:repo/commits/:sha-1 covered as well
in that to allow comments to fetch and submit.

It looks I confused API sets to think the submit functionality exists plain SHA-1 commits.
Finally I recovered the page I meant for the submitting:
https://developer.github.com/v3/repos/comments/#list-commit-comments

POST /repos/:owner/:repo/commits/:commit_sha/comments

So it must be feasible to implement. I may entertain myself in doing so :-)

Thanks for this wonderful package!

diff-mode, the parent of github-review-mode supports outline mode to show/hide hunks while inside the buffer. It seems like github-review-mode somehow broke this.

Maybe it's just my setup at work with github enterprise.

I set my github-review-host to "git.mycompany.com" but I needed to use api.git.mycompany.com in order to get things to work.

Otherwise, it bombed out with an unhelpful Got an error from the GitHub API (wrong-type-argument stringp nil)! even though it never sent the request as ghub--auth was erroring out.

Actually, maybe there's a bug somewhere. Because when I call gitreview-submit-commet it uses git.mycompany.com as the host.

I can't quite follow the logic of github-review-submit-review but I can't see it calling either github-review-post-review-replies or github-review-post-review when I haven't added any additional comments and I just want to approve the PR. Some modest instrumenting of the two legs and I see:

Submitting review, this may take a while ...
ghr-prr: t nil
ghr-rpr: nil nil

github-review-forge-pr-at-point works from magit-status view, but when you're visiting a PR (when you press RET and it opens it in forge-topic-mode) it does not work.

Hi there.

Is there a website for this repo?

Because if you don't have, well, this repo can simply be turned into a website right away. Others will discover this project in that website.

Steps:

1. Go to Settings and look for GitHub Pages, scroll down. That's almost at the bottom.

2. You will see there: Branch:none, so you should change that to master because you have a README.md file in the master repo. This will be your page. Click Save first.

3. Then click Choose a theme, you select a predefined theme of your site.

4. Visit your site now! The URL will be https://charignon.github.io/github-review.

If you were amazed by that, simply read the documentation about GitHub Pages.

FIrst of all, thank you for this amazing package.

Please document what token scopes github-review requires.

The README.md under Configuration says, I quote - github-review needs a GitHub token to act on your behalf for fetching PRs and submitting reviews. Create a personal access token using GitHub.... After that the document should be clearly stating what scopes github-review requires in order to function properly.

Please see the Token Creation at the magit/forge for the reference.

Thank you in advance.

Thanks for this package! I've been wanting to make something like this for a while.

It would be nice if the package can fetch "upstream" or "origin" (whatever the remote is called) and checkout the branch so that while viewing the diff, the branch is checked out which means i can open the file and see the code around.

In Haskell line comments are

-- this is a comment

and if a line is removed in a diff it looks like

--- this is a comment

which confuses the filename detector logic in github-review-non-null-filename-hunk-line

It all looks ea2f1b4 Use GraphQL API broke authentication for my environment (emacs 28 + ubuntu 18.04).
that was content with documented

machine api.github.com ...

I did not bisect it but supposedly after the commit I saw an error

Error talking to GitHub: ((error http 401 ((message . Bad credentials) (documentation_url . https://docs.github.com/rest)))

which is worked around with appending the path.

Line endings are inconsistent (mix between CRLF and LF) when displaying review buffer on Linux, which makes it ugly:

I don't know what it looks like on other platforms, feel free to comment if you also observe that behavior and are not using linux. If anyone wants to tackle it, make sure you read (http://ergoemacs.org/emacs/emacs_line_ending_char.html).

First off, thanks a lot for your great work on fixing #5!

I've enabled the feature already, and I'm trying to review this PR: bookingcom/shipper#70

If you try reviewing that, you'll see a lot of lines like this:

~ Reviewed by @isutton[COMMENTED]: 
~ Reviewed by @isutton[COMMENTED]: 
~ Reviewed by @isutton[COMMENTED]: 
~ Reviewed by @isutton[COMMENTED]: 

Strangely, there's also a line like this:

~ Reviewed by @icanhazbroccoli[COMMENTED]: 

Maybe this is some sort of a reply, since @icanhazbroccoli is the author and hasn't actually reviewed his own PR?

Thanks for making my life easier! :-)

That makes diagnosing problems very difficult.

Hello there, thank you for this absolutely amazing package!

Is it even possible to write multi-line code comments? The idea would be to select a segment of the diff and comment on it, kind of how you can do the same on GitHub.

Perhaps this is best left after the mode support, as this functionality could be a shortcut after selecting a part of the diff.

When I create a pull request from Emacs (personally I like to do C-c C-n from forge), it would also be nice if I could immediately request reviewers.

Similarly, it would be nice to be able to assign myself to the PR.

On attempt to github-review-comment, throws "Error talking to GitHub"

Magit Delta is an amazing package that gives you github-style diffs right in your Magit buffers. It would be amazing if github-review rendered sections in such a way that magit-delta-mode just worked. ™️

As an aside - this is a fantastic package, thanks for the work!

Hi

Thank you for that package.

M-x github-review-start mfg92/hugo-shortcode-gallery#15

I only see the repo's owner top comment

Thank you very much for your PR. I think...

I can not see the subsequent comments e.g.

Should this...

It would be nice if one could edit comments in gfm-mode (provided by markdown-mode). Then you could leverage markdown-modes support for Github Flavoured Markup.

As multiple modes in the same buffer is tricky, a simple(-ish) solution would be to open an indirect buffer narrowed to the comment in question.

It would require a minor mode for the diff buffer, a new way to mark comments (delimiters rather than prefixing each line with #) and would change the workflow somewhat (opening a new buffer for each comment), but I think having full gfm support is worth it. It would also closer mimic the click-a-line-get-a-textfield experience of Github.

It should submit a review for the first line instead in that case and not error out.

Deleted files seem to have the header /dev/null

Put the cursor over a commit and try to review it as a PR. You'll see an ugly error with this stack trace:

Debugger entered--Lisp error: (wrong-type-argument (or eieio-object class) nil obj)
  signal(wrong-type-argument ((or eieio-object class) nil obj))
  #f(compiled-function (obj slot) "Return the value in OBJ at SLOT in the object vector." #<bytecode 0x44ee7eb1>)(nil number)
  eieio-oref--closql-oref(#f(compiled-function (obj slot) "Return the value in OBJ at SLOT in the object vector." #<bytecode 0x44ee7eb1>) nil number)
  apply(eieio-oref--closql-oref #f(compiled-function (obj slot) "Return the value in OBJ at SLOT in the object vector." #<bytecode 0x44ee7eb1>) (nil number))
  eieio-oref(nil number)
  github-review-forge-pr-at-point()
  funcall-interactively(github-review-forge-pr-at-point)
  call-interactively(github-review-forge-pr-at-point)
  +magit/start-github-review(nil)
  funcall-interactively(+magit/start-github-review nil)
  call-interactively(+magit/start-github-review nil nil)
  command-execute(+magit/start-github-review)

This isn't really a bug, but the error message could be improved I think.

I searched and found #68 but I'm not trying it with a github enterprise repo, I'm doing:

M-x github-review-start https://github.com/codygman/devos/pull/1 RET

Then I receive:

Got an error from the GitHub API (wrong-type-argument stringp nil)!

I'm pretty sure Github isn't giving elisp errors 😉

First of all, thank you a lot for this package. The Github code review interface sucks when using a screen reader, but this allows me to use it with Emacs and Emacspeak. Awesomeness!

One problem right now is that I don't see existing comments as I'm reviewing the code. It'd be great to see the existing comments as i'm reading.

Code review is applied to the latest commit assuming that no code had change since it was fetched. This isn't always the case and we need to fix that assumption otherwise code review may refer to the wrong lines in the diff or fail to submit altogether.

I'm not even sure how this would work after glancing over the code but I've been enjoying magit-delta so thought I'd throw this idea up here.

It would be nice if diff-goto-source (C-c C-c) worked when this is invoked from github-review-forge-pr-at-point. All it needs to do is set the current directory to the git working tree in the diff-mode buffer. In many cases it does not matter too much whether the user has the "correct" branch is checked out - it just needs to be close.

It would be great to be able to use this with gitlab as well.

because I have customized diff-default-read-only to t, I get this error when trying to start a review:

Got an error from the GitHub API (buffer-read-only #<buffer val-lang___val___161___4ad8b9e769361183b16ad534847a7cdcad3b50ea.diff>)!

The instructions in the readme say to add:

(package!
 github-review
 :recipe
    (:fetcher github
     :repo "charignon/github-review"
     :files ("github-review.el")))

to your doom config. I'm running the development branch on emacs 28 and this produced an error for me:

Type: doom-package-error
  Message:
    Error with packages
  Data:
    (:private . packages)
    (doom-package-error "github-review" . "Keyword argument :fetcher not one of (:local-repo :files :flavor :no-build :type :repo :host :branch :remote :nonrecursive :fork :depth)")

Henrik (doom maintainer) says to use :host instead:

(package!
 github-review
 :recipe
    (:host github
     :repo "charignon/github-review"
     :files ("github-review.el")))

This fixed the error.

Currently, when it starts and generates diff-buffer, it puts the cursor at the end of the buffer. I think it should start at the top.

See minitest/minitest#793 for an example

It would be nice if, on the overview page, the comments read top-to-bottom with what was written up. They seem to submit from top-to-bottom and as such, read upside down (because newest first?).

It seems doom-emacs now uses code-review with magit by default (since doomemacs/doomemacs#5846). So the readme info seems to need revisions. I killed some time trying to figure out why I couldn't run the functions described.

It's about this part:

Add a line to your auth source files with your login and token: [...]

Bluntly, I just don't know what "my auth source files" refers to in this context :)

It would be nice if github-review could support replies to inline comments posted by others.

It would be great if the package can also show the list of files that have been changed. Also the file names / hunks should be clickable to be able to navigate to the file's place of change quickly.

Edit: This is similar to what magit offers

Referring to https://github.com/charignon/github-review#configuration

What levels of access should I permit on my github token for github-review to work?

Example of faulty behavior:

My Magit Forge is configured to work with my company's Github Enterprise. I just created a token for github-review and stored it in ~/.authinfo.gpg, but trying to github-review-forge-pr-at-point (when on a Forge PR) sends me into the buffer to interactively create a token on "api.github.com".

First of all: Thank you for creating this. I hope this will improve and would allow me to do all PR reviews without ever leaving Emacs.

I am aware that https://github.com/magit/forge has plans to integrate something like this, I hope this project maybe could become sort of a plugin. In the meantime maybe it would be possible to have minimal integration - one example would be to start a review without prompting for a url if cursor is already on a PR (given that user has Forge enabled).

I have an existing token with repo permissions in ~/.authinfo that I use with forge. However, this github-review seems to be unable to use it and asks if I want to create a new token.

Not sure if this one is easy to implement, but i think an interface similar to Github's side-by-side view helps alot for people like myself who like to view the old & new code in a vertical split