vscode-pull-request-github icon indicating copy to clipboard operation
vscode-pull-request-github copied to clipboard

Show Changes Since Last Review should not consider pending reviews as "Last Review"

Open ddrinka opened this issue 1 year ago • 1 comments

  • Extension version: 0.96.0
  • VSCode Version: 1.93.0
  • OS: Windows_NT x64 10.0.22621
  • Repository Clone Configuration (single repository/fork of an upstream repository): Single Repository
  • Github Product (Github.com/Github Enterprise version x.x.x): Github.com

Steps to Reproduce:

  1. Submit a review on a PR
  2. Add new commits to PR
  3. Use the "Show Changes Since Last Review" button to begin reviewing latest changes
  4. Begin a review
  5. Add another new commit to the PR
  6. Pull the new changes into the review

Expected Behavior: The union of the commits added in 2. and 5. should now be visible. Those commits represent the changes "since the last review".

Actual Behavior: Upon pulling the new changes, only the commits from 5. are visible. The pending review is considered the "last review", even though it was never submitted. In my case, the new changes were pulled automatically while I was in the midst of a review, and the change list reset to the most recent commit, with no way to go back to the state of the code I was in the middle of reviewing.

Proposal:

  • Only consider submitted reviews for the "Show Changes Since Last Review" functionality.
  • Support selecting a set of commits and view changes to files from those commits (similar to GitHub.com functionality), so that I can manually get back to reviewing changes since a point in time.

ddrinka avatar Sep 11 '24 21:09 ddrinka

I just realized the current behavior is even more difficult to use. Adding a new commit and refreshing just the PR view (which appears to happen automatically in some cases) triggers the above behavior of resetting the last-reviewed state to immediately before the newest commit. I didn't even have to click the button to pull the latest commit into the branch. The PR's current state and the code's state get disconnected in this case, which can lead to a lot of confusion, and seriously impact any ongoing review. The PR view should not consider any commits made after the one currently residing in the active branch, or else I should be able set the beginning and end commits to those I want to see during a single review session.

ddrinka avatar Sep 20 '24 17:09 ddrinka