Diffs between commits within a Github PR

[neilpeterman]

It's useful to zoom in on specific changes in response to review comments. Is there a way to do this?

[amit1rrr]

Review comments are on a specific notebook cell. The UI shows that cell just above the review comment. Do you wish to see more context around that cell or the entire commit diff?

Individual commit diffs are available separately. Would it help if we create easy navigation from PR discussion to commit diffs?

If possible, can you describe your workflow for this use case a little. Maybe it'll help us come up with a better/generic solution.

[neilpeterman]

Thanks for your reply. To clarify, I meant general comments that e.g. appear on the "Discussion" tab. These are often followed by changes to specific cells. What I see in the "Changes" tab is all of the commits together, for example for a new notebook this won't show diffs at all for the revisions.

If diffs for individual commits are available from the discussion tab that would be very helpful, akin to how this works on the "Conversation" tab on Github. Curently I don't see how to do that on ReviewNB -- when I click "XX commits" it navigates back to Github.

Two use cases we have that might help add context:

• New notebook PR
  1. Dev submits new notebook in a Pull request
  2. Reviewer makes general comments, for example -- "Some code is repeated and should be packaged into functions for better code quality / re-usability"
  3. Dev revises the notebook in subsequent commits to address these comments, but now it's useful for the Reviewer to see the specific changes that might only affect a small number of cells.
• Mixed format PR
  1. Dev submits PR that includes 2 notebooks and some python modules that are used by both notebooks.
  2. Reviewer comments on notebooks, but also in-line on github about specific functions in the python modules
  3. After recommended changes to modules, some changes are also necessary within the notebooks and are made in subsequent commits. It's useful for the reviewer to be able to zoom into these specific changes.

Either of these use cases is covered on some level by a tool like nbdime, but having this web-based like what you've built offers a lot of advantages.

[amit1rrr]

Thank you for explaining with use cases. It makes sense for us to have commit diffs so reviewer can identify & review only what was changed in a specific commit.

It'll be best to have a "Commits" tab (next to Discussion & Changes) that lists all commits in the PR. Clicking on any of them would show the commit diff. How does that sound?

[neilpeterman]

I think that would work really great!

[amit1rrr]

Cool. I have noted this down, expect it to be implemented within the next few weeks. Let's keep this issue open until then.

[amit1rrr]

@neilpeterman This is now live. There's a Commits tab under the PR which will help you quickly see commit diffs. Let me know if you face any issues accessing it.

[neilpeterman]

@amit1rrr Looks awesome!