Commit-wise PR reviews

[pwntester]

Add support for commit-wise reviews by adding a new command Octo review commit which can be used after starting or resuming a PR review. This command will show the commits forming the PR and upon selection, will filter the diff panel to only show the files changed on that particular commit. The commit sha will be used to fetch the right file version from GitHub or from the local git repo. Adding/replying to comments should work as before. Comments/replies are created as Pending and need to be submitted to finalize the review.

[deathmaz]

for me nothing happens when i run Octo review commit command, in video i ran it 2 times:

https://user-images.githubusercontent.com/6440135/169044715-5f642e67-47d5-4c27-8bc9-351bf273fc48.mp4

i used commit_wise_reviews branch

EDIT: some additional info:

❯ nvim -v
NVIM v0.8.0-dev
Build type: Release
LuaJIT 2.1.0-beta3
Compiled by root@a232bc0491d7

Features: +acl +iconv +tui
See ":help feature-compile"

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "/app/dist/share/nvim"

Run :checkhealth for more info

call plug#begin()
  Plug 'pwntester/octo.nvim', { 'branch': 'commit_wise_reviews' }
call plug#end()

lua << EOF
require"octo".setup()
EOF

[pwntester]

@deathmaz you need to start a review first either Octo review start or Octo review resume if you have one started already. Any how, I found a little bug, so Octo review commit should work for filtering the review panel, but placing comments will fail. Will fix ASAP

[pwntester]

@deathmaz please give it another try and let me know if it works for you

[deathmaz]

I tried and now i'm able to pick the commit for review. Few things that i noticed so far:

it's not possible to add multi line comments


after adding non multi line comment there's no comment icon in the guttuer


after running `Octo review comments` and selecting the comment there's an error


[pwntester]

it's not possible to add multi line comments

Yep, unfortunately, this is not yet supported by the API. There is an option to add a multi-line commit-level comment using the REST API, but it sends the comment straightaway instead of making it a pending comment which I think is a better approach for commit-level reviews (since you can always change your mind about that comment after reviewing a different commit). Let me know if the non-review multi-line comments is something you would like to get implemented.

after adding non multi line comment there's no comment icon in the guttuer

Thats weird, you should get the icon and the virtual text as in

Will look into it

after running Octo review comments and selecting the comment there's an error

Again, this is working for me 🤔 The error seems to appear when you select a comment. Can you try with latest version?

[deathmaz]

now i'm getting error when adding new comment

https://user-images.githubusercontent.com/6440135/169271421-1dc08f28-4e7f-40c5-a8c0-e0dcbf58940c.mp4

[pwntester]

@deathmaz thanks for reporting, that was a different bug not allowing commit-level comments on newly added files (rather than modified) since there is no diff associated to them. Should be fixed now

[deathmaz]

i'm able to add new comments and i see comment icon in the gutter now. Also i don't have error when jumping to comment with Octo review comments command. But i noticed that when reviewing specific commit sometimes it doesn't jump exactly to the line with the comment. Details in the video:

https://user-images.githubusercontent.com/6440135/169279418-f5402e0c-cd14-418b-a09d-36fb87105422.mp4

I added two pending comments in a220013 commit. While i'm not in commit review mode it jumps exactly to the line with the comment. Then i switch to the review of the commit on which i added the comments and in there when i try to jump to comment in lua/octo/reviews/thread-panel.lua file it jumps above the line with the comment.

[pwntester]

While i'm not in commit review mode it jumps exactly to the line with the comment. Then i switch to the review of the commit on which i added the comments and in there when i try to jump to comment in lua/octo/reviews/thread-panel.lua file it jumps above the line with the comment.

Should be fixed now. Github remembers both the original position (at the commit where the comment was placed) and the final position in the PR. If you move to a different commit where the commented line was moved to a third location, the jump will move to the wrong position. In that case, I can make it jump to the PR position or fail with a message ("comment does not belong to this commit" or something similar), what would you prefer?

[deathmaz]

Should be fixed now.

yes, works now.

If you move to a different commit where the commented line was moved to a third location, the jump will move to the wrong position. In that case, I can make it jump to the PR position or fail with a message ("comment does not belong to this commit" or something similar), what would you prefer?

Probably make it jump to the PR position. Otherwise, if it's not possible to jump to that kind of comments there's probably also no need to display them in the list of Octo review comments command but displaying only comments related to that commit. Or maybe there should be a separate command/flag to display only comments for current commit and separate for comments for the whole PR.

[deathmaz]

noticed an error when trying to submit a review after pressing <C-a>:

[pwntester]

@deathmaz sorry, there was a typo that I did not test properly :( Can you try again?

[deathmaz]

@pwntester works now 👍🏻

[pwntester]

Thanks a lot for testing it @deathmaz Im gonna merge this now but please report any issues you may find

[weilbith]

👍🤩🤗😍🤙🚀