Skip to content

Add repo list command#2953

Merged
mislav merged 10 commits intocli:trunkfrom
cristiand391:add-repo-list
Feb 27, 2021
Merged

Add repo list command#2953
mislav merged 10 commits intocli:trunkfrom
cristiand391:add-repo-list

Conversation

@cristiand391
Copy link
Contributor

@cristiand391 cristiand391 commented Feb 11, 2021

Closes #642

This PR adds support for listing repositories of a user or organization, some examples:

TTY Output
Screenshot_2021-02-25_12-08-30

Non-TTY Output
Screenshot_2021-02-25_12-09-01

UI Spec: https://docs.google.com/document/d/1QoktdMTS8FEXYLH_1WvzZe_OoGV_ZiTxf0c3QVF1soY/edit#

@cristiand391 cristiand391 changed the title Add repo list command [WIP] Add repo list command Feb 11, 2021
if notArchived && repo.IsArchived {
matchCount--
listResult.TotalCount--
continue
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When doing gh repo list --fork or gh repo list --source, don't list archived repos.
This mimics the way GitHub does filter repos in the web UI.

@mislav
Copy link
Contributor

mislav commented Feb 12, 2021

@cristiand391 Thanks for picking this up! This looks great already.

I'm going to suggest some output tweaks that are different from the design spec you were working off of, and ask input from my team to see if any of this resonates with them:

  • I'm not sure if we need the --archived filter. Can anyone think of a use-case for it? I don't think we need to necessarily duplicate every filtering option available in the web UI— at least not for now.
  • “When doing gh repo list --fork or gh repo list --source, don't list archived repos.” - Even if the web UI does this (TIL!), should we also do it? Since this command will have programmatic use, maybe it's best to avoid encoding some magic behavior. If we anticipate the need for avoiding archived repos, we could expose a flag such as --no-archived?
  • Could repo tags such as “Private, archived" be rendered into the 3rd column (instead of the 2nd), not be placed in parentheses (we only do this for repo labels), and be separated by commas? This way, the first 2 columns will be repo name & title, which are in my view the most important information.

@ampinsk
Copy link

ampinsk commented Feb 12, 2021

I'm not sure if we need the --archived filter. Can anyone think of a use-case for it? I don't think we need to necessarily duplicate every filtering option available in the web UI— at least not for now.

I'm honestly agnostic on this one! If there's no strong push for it, I'm fine to punt on it.

“When doing gh repo list --fork or gh repo list --source, don't list archived repos.” - Even if the web UI does this (TIL!), should we also do it? Since this command will have programmatic use, maybe it's best to avoid encoding some magic behavior. If we anticipate the need for avoiding archived repos, we could expose a flag such as --no-archived?

I don't feel super strongly about this either. But, even for programatic use cases, I can imagine if you archive a repository, it might be a lot of clutter to see them showing up in lists by default? And as a get around, wouldn't passing both --fork and --archived show archived forks for example?

Could repo tags such as “Private, archived" be rendered into the 3rd column (instead of the 2nd), not be placed in parentheses (we only do this for repo labels), and be separated by commas? This way, the first 2 columns will be repo name & title, which are in my view the most important information.

I like this change! 👍

@billygriffin
Copy link
Contributor

I'm not sure if we need the --archived filter. Can anyone think of a use-case for it? I don't think we need to necessarily duplicate every filtering option available in the web UI— at least not for now.

+1. I definitely don't think this is needed unless we actually see some significant demand for it.

When doing gh repo list --fork or gh repo list --source, don't list archived repos.” - Even if the web UI does this (TIL!), should we also do it? Since this command will have programmatic use, maybe it's best to avoid encoding some magic behavior. If we anticipate the need for avoiding archived repos, we could expose a flag such as --no-archived?

I have no strong opinions about this either. Honestly I don't think it matters that much, and therefore would opt to mirror what dotcom does, especially since that helps reduce clutter as @ampinsk pointed out.

Could repo tags such as “Private, archived" be rendered into the 3rd column (instead of the 2nd), not be placed in parentheses (we only do this for repo labels), and be separated by commas? This way, the first 2 columns will be repo name & title, which are in my view the most important information.

This is 💯

@cristiand391 cristiand391 marked this pull request as ready for review February 25, 2021 15:11
@cristiand391 cristiand391 changed the title [WIP] Add repo list command Add repo list command Feb 25, 2021
@cristiand391
Copy link
Contributor Author

@mislav I've added some tests and pushed some changes to address suggestions.
Regarding to listing archived repos when filtering, should I remove it or add some new flag or something else?

Also order results by PUSHED_AT instead of UPDATED_AT to match the web
interface.
Dynamically construct the GraphQL query by using the `viewer` connection
if the owner isn't set and the `repositoryOwner(login:"...")` connection
if the owner was set.
Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for all your work!

I've pushed changes to:

  1. Avoid having to separately request the current username when owner isn't specified via argument;
  2. Remove all logic that filters out archived repos, since this level of filtering isn't supported in the API layer;
  3. Sort the returned results by PUSHED_AT instead of UPDATED_AT to match the web interface;
  4. Enable the pager for tty output.

@data-man
Copy link

Feature request: filtering by language.
Currently I'm using this tiny script:

set -e

repos() {
  local owner="${1?}"
  shift 1
  gh api graphql --paginate -f owner="$owner" "$@" -f query='
    query($owner: String!, $per_page: Int = 100, $endCursor: String) {
      repositoryOwner(login: $owner) {
        repositories(first: $per_page, after: $endCursor, ownerAffiliations: OWNER) {
          nodes {
            nameWithOwner
            description
            primaryLanguage { name }
            isFork
            pushedAt
          }
          pageInfo {
            hasNextPage
            endCursor
          }
        }
      }
    }
  ' | jq -r '.data.repositoryOwner.repositories.nodes[] | [.nameWithOwner,.pushedAt,.description,.primaryLanguage.name,.isFork] | @tsv' | sort
}

repos "$@"

@mislav
Copy link
Contributor

mislav commented Feb 27, 2021

@data-man That's an excellent suggestion. Added!

Note that for filtering by language, we have to turn to the search API connection (as opposed to the repositories connection). In that case, the results are sorted in updated-desc order rather than in PUSHED_AT order.

Like `--language`, archived filters also use the Search API.
@data-man
Copy link

Proposals:

-a      --archived          Show only archived repositories
-f      --fork              Show only forks
-n      --no-archived       Omit archived repositories
-s      --source            Show only non-forks

@mislav
Copy link
Contributor

mislav commented Feb 27, 2021

@data-man Thanks, but I do not think that every long flag needs to have a shorthand. In particular, -f and -s don't read really well when on their own, as opposed to -l ruby which is likely self-evident. Therefore, I'd rather that people spell out --fork and --source.

@mislav mislav merged commit 00cb921 into cli:trunk Feb 27, 2021
@data-man
Copy link

@mislav

It would be nice to show a primary language as well.

@cristiand391 cristiand391 deleted the add-repo-list branch February 27, 2021 22:43
Copy link

@Gpapi13 Gpapi13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N and P

Copy link

@bruna17 bruna17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Details

Copy link

@cuddytrippin cuddytrippin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pull requests #3061 `

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gh repo list

8 participants