Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to mark a file as "reviewed" #520

Closed
bpasero opened this issue Sep 28, 2018 · 19 comments · Fixed by #2664
Closed

Allow to mark a file as "reviewed" #520

bpasero opened this issue Sep 28, 2018 · 19 comments · Fixed by #2664
Assignees
Labels
feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Sep 28, 2018

My workflow with pull requests goes typically like this:

  • download a patch of the change
  • apply it locally
  • go to the changes view
  • review a file
  • once done with a file, stage it

I am missing the last part and this blocks me from using this extension in my day to day workflow for more complex changes.

@Tyriar
Copy link
Member

Tyriar commented Sep 28, 2018

@bpasero can you not just review the files in order and keep track of which file you're up to?

@bpasero
Copy link
Member Author

bpasero commented Sep 28, 2018

@bpasero I like to jump around a lot when reviewing and typically go in order that is not reflected by the alphabetic sort order

@justinliew
Copy link
Contributor

I may look into this, as I definitely like to use this sort of workflow when doing PRs.

@RMacfarlane RMacfarlane added the feature-request Request for new features or functionality label Oct 1, 2018
@RMacfarlane
Copy link
Contributor

I also review files out of order and would like to see this. Some other code review tools let you mark things off in the file changes section:
reviewed_files

I think it would be cool to have something similar in our tree view, where there's some gesture for marking the file as reviewed that updates the decoration of that tree item.

@justinliew
Copy link
Contributor

Cool, that's great inspiration! I will start looking into this.

@justinliew
Copy link
Contributor

If anyone wants to take this one, I'm having a bit of trouble as the decoration type UI stuff I'm super unfamiliar with. I will keep looking at it if nobody else does but I'm taking on some easier ones to get more familiar with the project first.

@Tyriar
Copy link
Member

Tyriar commented Oct 4, 2018

@justinliew this one is probably best to go through our UX people first as it's not clear what the flow will look like.

@justinliew
Copy link
Contributor

Sounds good. I'll keep watching this one but leave the design for the people with the know-how/expertise to ensure consistency.

@mrlubos
Copy link

mrlubos commented Mar 17, 2019

Hello, are there any updates on this? +1, would love to use this feature!

@doupp-adsk
Copy link

This gets a +1 from me as well.

@doupp-adsk
Copy link

Coming back to this to point out that there is UI already in the web to do something like this. Details are here:

https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/reviewing-proposed-changes-in-a-pull-request#marking-a-file-as-viewed

Mark as read on github.com

I could easily see this as a checkbox in the tree view.

@dentuzhik
Copy link

This issue relates to this issue as well, and for me I have to review PR in two places, one in VSCode because of a nice tree structure and editor, and another in Github to mark file as viewed. Would be great to have it in VSCode.

Can somebody maybe point to the place in source where one might think implementing this?
\cc @RMacfarlane sorry for direct ping, maybe you can help :)

@RMacfarlane
Copy link
Contributor

Hey @dentuzhik!

There isn't a way to render a checkbox in the tree, but we could show a checkmark decoration next to files that are marked as viewed. In the below screenshot, the diamond and letter indicating the git status are tree decorations:
Screen Shot 2020-02-04 at 10 22 19 AM
These are implemented here and here.

We could have an inline action or an entry in the context menu for marking the file as viewed, there are some examples of that here.

I just tried looking through the GitHub API docs (both v3 and v4) to see what calls we would need to make to get which files are already marked as viewed, and I wasn't able to find anything about it. I'm not sure if the API is not publicly available, or if I just missed it. Let me know if you're able to find it! I'd love to take a PR for this.

@dentuzhik
Copy link

Much appreciated help, thank you 🙌
I will look over the code and see if I can make something work :)

@dentuzhik
Copy link

I'm glad that this got into the backlog :D

A year has passed, but it seems that there's actually a v4 version of this API available, so 🤞🏼 we'll see this some time soon!
https://docs.github.com/en/graphql/reference/mutations#markfileasviewed

@RMacfarlane
Copy link
Contributor

Viewed files are now decorated with a checkmark, and there's an inline action to mark them as viewed/not viewed:
mark-as-viewed

@jillesmc
Copy link

@RMacfarlane. Could you tell me how can I enable that?
I'm using version 0.25.1, but there is no inline actions for me.

@RMacfarlane
Copy link
Contributor

This hasn't been released yet, we follow the same shipping schedule as VS Code, so should have a new version two weeks from now.

In the meantime if you want to try it out, you can use the Nightly version of the extension: https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github-insiders

(Note that the Nightly and Stable versions of the extension can't be installed side by side since their commands will conflict, so you will have to uninstall the Stable version to use Nightly)

@jillesmc
Copy link

Thank you @RMacfarlane. You guys are doing an amazing job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants