Skip to content

Add relevant metadata to issue view and pr view #663

Closed
Closed
Add relevant metadata to issue view and pr view#663
Assignees
doi-tampinsk
Labels
enhancementa request to improve CLI

Description

@billygriffin
billygriffin
opened on Mar 16, 2020

Describe the feature or problem you’d like to solve

When using the terminal view for gh issue view and gh pr view (currently requiring the --preview flag but soon to be the default), there's a lot of metadata that does not show up in the output that would be helpful to people.

Proposed solution

Mirroring most of the information in the issue/PR sidebar seems sensible as a starting point, but I have no opinion about how it should be presented.

cc: @ampinsk

Additional context

We're removing the labels from immediately below the header in #652, so this will be more useful when that ships.

Activity

  1. billygriffin
    added
    enhancementa request to improve CLI
    pull request
    issue
    on Mar 16, 2020

  2. ampinsk commented on Mar 18, 2020

    @ampinsk
    ampinsk
    on Mar 18, 2020

    Screen Shot 2020-03-18 at 2 08 17 PM

    Here's where we landed on how to display sidebar info!

    Some notes:

    • As you can see in the screenshot, field names should be bolded
    • Metadata should be shown inbetween the header info and the body
    • When a field is empty, don't display it
    • Let's not show the notifications field at all for now

  3. billygriffin commented on Mar 23, 2020

    @billygriffin
    billygriffin
    on Mar 23, 2020
    ContributorAuthor

    @doi-t Since this is in line with the things you've been contributing to date, do you have interest in picking this up? Please don't feel any obligation whatsoever, but we wanted to give you the first opportunity since you've recently worked on similar things and have made such valuable contributions.

  4. doi-t commented on Mar 24, 2020

    @doi-t
    doi-t
    on Mar 24, 2020
    Contributor

    @billygriffin
    Yay! Thank you for the opportunity! 😄I'm definitely interested in it. Will work on it. 🙌

  5. doi-t commented on Apr 2, 2020

    @doi-t
    doi-t
    on Apr 2, 2020
    Contributor

    @billygriffin
    I have some things to make sure:

  6. ampinsk commented on Apr 2, 2020

    @ampinsk
    ampinsk
    on Apr 2, 2020

    Never mind! I guess #663 (comment) is an example of no milestone which follows a note (When a field is empty, don't display it) and we want to show a milestone title

    Hi @doi-t, yes you figured that out exactly right! Apologies for that not being immediately clear 🙇‍♀

    Could you please assign you and me to this issue? 🙏 Then, this issue can be an example of multiple assignees for gh issue view 663 --repo cli/cli

    I can do this!

    Does Github GraphQL API v4 already support Linked pull requests and Linked issues? IIRC, it is a relatively new feature on github.com 🤔

    @mislav @vilmibm should be able to give you a better answer on that! From my perspective, if it's not easy to implement then I'm totally fine leaving it out for now.

  7. ampinsk
    assigned
    doi-t
    and
    ampinsk
    on Apr 2, 2020

  8. doi-t commented on Apr 4, 2020

    @doi-t
    doi-t
    on Apr 4, 2020
    Contributor

    @ampinsk Thank you for your guide and the issue assignment! 😄

    Reviewers in pr view

    I made a PR #748 for pr view. Through the implementation, I realized that we haven't discussed Reviewers and its state, which is the most useful metadata for me at least. 🤩

    Design Perspective

    @ampinsk
    Following the basic principle of mirroring GitHub.com in CLI preview, I assume that we basically want to copy what Web UI does. As far as I know, we see 4 types of reviewers' state along with a short message in Web UI. How should I display the state of reviewers in CLI? My quick thought is foo (Approved), bar (Changes requested) just like Projects. Please let me know if my assumption and understanding are wrong. 🙇

    Screen Shot 2020-04-04 at 13 44 17
    Screen Shot 2020-04-04 at 13 44 40
    Screen Shot 2020-04-04 at 13 45 05
    Screen Shot 2020-04-04 at 13 48 11

    Technical Perspective

    @mislav @vilmibm
    It seems that Reviewers is more complicated than other metadata. There are ReviewRequest and PullRequestReview. If I understand it correctly, we need to individually calculate both requested reviewers (RequestedReviewer) and the latest state of reviewers (APPROVED, CHANGES_REQUESTED, COMMENTED and others).

    I tried to collect reviewers' information with doi-t/cli@85d8f80...doi-t:8fca78f and turned out that it also includes the history of reviews besides above multiple factors.

    Below partial JSON data is an example of ReviewRequests. We receive it only when someone requests a review.

                "reviewRequests": {
              "nodes": [
                {
                  "requestedReviewer": {
                    "__typename": "User",
                    "login": "vilmibm"
                  }
                }
              ],
              "totalCount": 1
            },

    Below partial JSON data is an example of gh pr view 667 --repo cli/cli. In this case, no one made a review request but the code review happened so that ReviewRequests is empty but Reviews has its history. As you can see below, last 3 events was one COMMENTED by the author which is not relevant as metadata and two APPROVED by the same reviewer which is duplicated information in terms of pr view. It also means that we need to look back at the review history as much as possible and calculate the latest states of each reviewer if there is no way to filter them.

                "ReviewRequests": {
                "Nodes": [],
                "TotalCount": 0
            },
            "Reviews": {
                "Nodes": [
                    {
                        "Author": {
                            "Login": "doi-t"
                        },
                        "State": "COMMENTED",
                        "CreatedAt": "2020-03-17T23:46:34Z",
                        "PublishedAt": "2020-03-17T23:46:34Z"
                    },
                    {
                        "Author": {
                            "Login": "mislav"
                        },
                        "State": "APPROVED",
                        "CreatedAt": "2020-03-31T15:26:48Z",
                        "PublishedAt": "2020-03-31T15:26:48Z"
                    },
                    {
                        "Author": {
                            "Login": "mislav"
                        },
                        "State": "APPROVED",
                        "CreatedAt": "2020-04-02T18:19:58Z",
                        "PublishedAt": "2020-04-02T18:19:58Z"
                    }
                ]
            },

    (Relevant discussion: https://github.community/t5/GitHub-API-Development-and/How-to-retrieve-PR-reviewers-with-GraphQL-API/m-p/20974)

    Based on above understandings, the idea I can come up with is:

    1. Query ReviewRequests and Reviews (e.g. last: 100)
    2. List up unique reviewers from ReviewRequests and Reviews and remove the PR author
    3. Generate reviewer list with the last state (e.g. foo (Approved), bar (Changes requested))
    4. If there are more than, for example, 3 reviewers, display the first 3 reviewers with their last state

    My understanding and approach itself could be wrong so that I appreciate your guide. 🙇 I'm really curious how GitHub.com internally calculate the latest state of reviewers for the Web UI. 🤔

  9. doi-t commented on Apr 5, 2020

    @doi-t
    doi-t
    on Apr 5, 2020
    Contributor

    📝 Found relevant implementations:

    Logic wise, it seems it is fine to overwrite reviewer's last states of Reviews by ReviewRequests.

    Will try to make a proof of concept to see the actual logic in Go. 👀

  10. doi-t commented on Apr 7, 2020

    @doi-t
    doi-t
    on Apr 7, 2020
    Contributor

    @ampinsk @billygriffin
    I made another PR for Reviewers #762. Please take a look at it when you have time.

    Here is the things needs to be confirmed for the design:

  11. ampinsk commented on Apr 7, 2020

    @ampinsk
    ampinsk
    on Apr 7, 2020

    @doi-t Thanks for the clear questions!

    • I'm comfortable omitting participants
    • Let's not truncate
    • For reviewers, could we put their review state in the following colors?

    Approved: Green
    Changes requested: Red
    Commented: No color
    Requested: Yellow

  12. ampinsk
    closed this as completedon May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementa request to improve CLI

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions