search

cli / cli

GitHub’s official command line tool
https://cli.github.com
MIT License
37.2k stars 5.77k forks source link

`gh search prs` includes prs from archived repos #8699

Open stuart-leitch opened 8 months ago

stuart-leitch commented 8 months ago

Describe the bug

Using gh search prs --state open --owner <OWNER> gets a list of all open PRs across repos in an organisation. However PRs from archived repositories are included. Adding --archived restricts the PRs returned to those in archived repositories, but I can see no way to exclude PRs in archived repos.

You can argue this is an enhancement rather than a bug, but it does seem counterintuitive. I'd expect the default behaviour to exclude archived repos, and the --archived switch to include them.

Additionally it does not seem possible to include whether the repo is archived in the returned data. An alternative fix (which avoids breaking existing behaviour) would be to add and 'archived' field the the allowable json fields so that these PRs can be filtered out post-search.

gh version 2.43.1

Steps to reproduce the behavior

  1. Create a github org 'ghreposearchprsarchived'
  2. Create a new repo
  3. Open a PR in this repo
  4. Archive the repo created in step 2
  5. Create a new repo
  6. Open a PR in this repo
  7. Install and authenticate gh cli for this org
  8. run gh search prs --state open --owner ghreposearchprsarchived
  9. Both PRs created above are listed (expected one)
  10. run gh search prs --state open --owner ghreposearchprsarchived --archived
  11. Only one PR is listed (from the archived repo)
stuart-leitch commented 8 months ago

Wee note here. I noticed that gh repo list has a similar thing, but also has a --no-archived flag which would be another option for the gh search prs subcommand. Consistent behaviour would be good :)

andyfeller commented 8 months ago

@stuart-leitch : thank you for opening up this issue! ❤

gh search prs --archived=false should do what you need

Whether to include archived repositories or not goes back to gh search commands being added in https://github.com/cli/cli/pull/5334#discussion_r834565376 but the good news is --archived=false works today:

To clarify: https://github.com/issues adds archived:false to the query by default. This is likely because results from archived repositories are less useful: they are read-only and thus cannot be be acted on. But if you don't think that this should be the default, that's totally fine! I agree about your point re: magicness. We can wait and see whether it will be cumbersome to add --archived=false to most search queries by hand.

For demonstration purposes, let's look at the `github` organization:

```shell $ gh search prs --owner github --visibility public --archived=true Showing 30 of 10105 pull requests REPO ID TITLE LABELS UPDATED github/globalmaintainersummit.github.com #339 Bump sass from 1.35.1 to 1.69.5 dependencies about 3 months ago github/globalmaintainersummit.github.com #338 Bump browserify-sign from 4.2.1 to 4.2.2 dependencies about 3 months ago github/codespaces-actions-playground #8 Update README.md to mention archive about 3 months ago github/globalmaintainersummit.github.com #337 Bump sass from 1.35.1 to 1.69.4 dependencies about 3 months ago github/globalmaintainersummit.github.com #336 Bump @babel/traverse from 7.14.7 to 7.23.2 dependencies about 4 months ago github/globalmaintainersummit.github.com #335 Bump sass from 1.35.1 to 1.69.3 dependencies about 3 months ago github/globalmaintainersummit.github.com #334 Bump webpack from 4.46.0 to 5.89.0 dependencies about 4 months ago github/globalmaintainersummit.github.com #333 Bump sass from 1.35.1 to 1.69.0 dependencies about 4 months ago github/globalmaintainersummit.github.com #332 Bump sass from 1.35.1 to 1.68.0 dependencies about 4 months ago github/globalmaintainersummit.github.com #331 Update index.json to remove reference to next year about 4 months ago github/globalmaintainersummit.github.com #330 Bump sass from 1.35.1 to 1.67.0 dependencies about 4 months ago github/globalmaintainersummit.github.com #329 Bump sass from 1.35.1 to 1.66.1 dependencies about 5 months ago github/globalmaintainersummit.github.com #328 Bump sass from 1.35.1 to 1.65.1 dependencies about 5 months ago github/globalmaintainersummit.github.com #327 Bump sass from 1.35.1 to 1.64.2 dependencies about 6 months ago github/globalmaintainersummit.github.com #326 Bump sass from 1.35.1 to 1.64.1 dependencies about 6 months ago github/globalmaintainersummit.github.com #325 Bump webpack from 4.46.0 to 5.88.2 dependencies about 4 months ago github/globalmaintainersummit.github.com #324 Bump word-wrap from 1.2.3 to 1.2.4 dependencies about 7 months ago github/eslint-plugin-custom-elements #59 Bump word-wrap from 1.2.3 to 1.2.4 dependencies about 7 months ago github/github-graphql-relay-example #36 Bump semver from 5.4.1 to 5.7.2 dependencies about 5 months ago github/globalmaintainersummit.github.com #323 Bump semver from 5.7.1 to 5.7.2 dependencies about 7 months ago github/globalmaintainersummit.github.com #322 Bump stylelint from 13.13.1 to 15.10.1 dependencies about 7 months ago github/globalmaintainersummit.github.com #321 Bump webpack from 4.46.0 to 5.88.1 dependencies about 6 months ago github/globalmaintainersummit.github.com #320 Bump sass from 1.35.1 to 1.63.6 dependencies about 6 months ago github/globalmaintainersummit.github.com #319 Bump webpack from 4.46.0 to 5.88.0 dependencies about 7 months ago github/globalmaintainersummit.github.com #318 Bump sass from 1.35.1 to 1.63.3 dependencies about 7 months ago github/globalmaintainersummit.github.com #317 Bump webpack from 4.46.0 to 5.86.0 dependencies about 7 months ago github/globalmaintainersummit.github.com #316 Bump webpack from 4.46.0 to 5.85.0 dependencies about 8 months ago github/testrepo #42 teteee3erefffffffteteee3erefffffffteteee3erefffffffteteee3erefffffffteteee3ereff... about 8 months ago github/VisualStudio #2734 Codespace sandeepdeepu24 bug free space funicular gg6qg4w6q7v2vpvv about 8 months ago github/globalmaintainersummit.github.com #315 Bump webpack from 4.46.0 to 5.84.1 dependencies about 8 months ago $ gh search prs --owner github --visibility public --archived=false Showing 30 of 83664 pull requests REPO ID TITLE LABELS UPDATED github/docs #31659 Repo sync triage about 1 hour ago github/docs #31658 Repo sync triage about 2 hours ago github/docs #31657 Repo sync triage about 2 hours ago github/advisory-database #3545 [GHSA-78xj-cgh5-2h22] NPM IP package vulnerable to Server-Side Request Fo... about 3 hours ago github/vscode-codeql #3368 Bump @testing-library/jest-dom from 6.2.0 to 6.4.2 in /extensions/ql-vscode Update dependencies about 3 hours ago github/vscode-codeql #3367 Bump @typescript-eslint/parser from 6.16.0 to 7.0.1 in /extensions/ql-vscode Update dependencies about 3 hours ago github/vscode-codeql #3366 Bump storybook from 7.6.10 to 7.6.15 in /extensions/ql-vscode Update dependencies about 3 hours ago github/vscode-codeql #3365 Bump mini-css-extract-plugin from 2.7.7 to 2.8.0 in /extensions/ql-vscode Update dependencies about 3 hours ago github/vscode-codeql #3364 Bump @storybook/addon-a11y from 7.6.13 to 7.6.15 in /extensions/ql-vscode Update dependencies about 3 hours ago github/docs #31656 Update index.md triage about 4 hours ago github/advisory-database #3544 [GHSA-g74q-5xw3-j7q9] Microsoft Security Advisory CVE-2024-21386: .NET De... about 4 hours ago github/explore #4236 Bump actions/jekyll-build-pages from 1.0.9 to 1.0.12 dependencies, github_actions about 7 hours ago github/docs #31653 Update working-with-the-npm-registry.md triage about 8 hours ago github/advisory-database #3543 [GHSA-xvc9-xwgj-4cq9] Integer Overflow in HeaderMap::reserve() can cause ... about 9 hours ago github/advisory-database #3542 [GHSA-xm6r-4466-mr74] OrientDB vulnerable to Improper Privilage Managemen... about 10 hours ago github/docs #31652 Repo sync triage about 13 hours ago github/docs #31651 Repo sync triage about 14 hours ago github/codeql-coding-standards #534 Fix FP reported in #388 about 14 hours ago github/advisory-database #3541 [GHSA-r48h-jr2j-9g78] aws/resource_aws_iam_user_login_profile.go in the... about 14 hours ago github/advisory-database #3540 [GHSA-r48h-jr2j-9g78] aws/resource_aws_iam_user_login_profile.go in the... about 14 hours ago github/docs #31650 Repo sync triage about 15 hours ago github/advisory-database #3539 [GHSA-r48h-jr2j-9g78] aws/resource_aws_iam_user_login_profile.go in the... about 14 hours ago github/docs #31649 Repo sync triage about 15 hours ago github/github-app-js-sample #36 fix fetch setup issue about 15 hours ago github/docs #31648 Repo sync triage about 16 hours ago github/transparency #4 Update EU DSA MAUs about 16 hours ago github/safe-settings #592 fix: hardcoded .github reference --> CONFIG_PATH about 17 hours ago github/safe-settings #591 fix(rulesets): Repo scope update trying to remove org rulesets about 2 hours ago github/safe-settings #590 fix(teams): Deprecated team repo endpoint about 17 hours ago github/dmca #15057 Added 2024/02/2024-02-07-source-code-4.md about 17 hours ago $ gh search prs --owner github --visibility public Showing 30 of 95038 pull requests REPO ID TITLE LABELS UPDATED github/codeql #15627 Move the JS java tests to be a proper `java_test` target. JS about 20 minutes ago github/codeql-action #2151 Account for existing but empty `cpus` file about 4 minutes ago github/docs #31659 Repo sync triage about 1 hour ago github/docs #31658 Repo sync triage about 2 hours ago github/docs #31657 Repo sync triage about 2 hours ago github/advisory-database #3545 [GHSA-78xj-cgh5-2h22] NPM IP package vulnerable to Server-Side Request Forger... about 3 hours ago github/vscode-codeql #3368 Bump @testing-library/jest-dom from 6.2.0 to 6.4.2 in /extensions/ql-vscode Update dependencies about 3 hours ago github/vscode-codeql #3367 Bump @typescript-eslint/parser from 6.16.0 to 7.0.1 in /extensions/ql-vscode Update dependencies about 3 hours ago github/vscode-codeql #3366 Bump storybook from 7.6.10 to 7.6.15 in /extensions/ql-vscode Update dependencies about 3 hours ago github/vscode-codeql #3365 Bump mini-css-extract-plugin from 2.7.7 to 2.8.0 in /extensions/ql-vscode Update dependencies about 3 hours ago github/vscode-codeql #3364 Bump @storybook/addon-a11y from 7.6.13 to 7.6.15 in /extensions/ql-vscode Update dependencies about 3 hours ago github/codeql #15626 C++: Don't strip specifiers away in `TFinalParameterUse` C++, no-change-note-required about 1 hour ago github/codeql #15625 C# 12: Primary constructor inititalizers. C# about 1 hour ago github/codeql-action #2150 Merge releases/v3 into releases/v2 about 3 hours ago github/codeql-action #2149 Mergeback v3.24.3 refs/heads/releases/v3 into main about 3 hours ago github/codeql-action #2148 Merge main into releases/v3 about 3 hours ago github/codeql #15624 Add new libraries we cover to frameworks.csv Go about 3 hours ago github/codeql-action #2147 Fix an error when loading a configuration specified by the `config` input about 3 hours ago github/docs #31656 Update index.md triage about 4 hours ago github/codeql #15623 C#: update the QHelp for `cs/web/unvalidated-url-redirection` C#, documentation about 4 hours ago github/advisory-database #3544 [GHSA-g74q-5xw3-j7q9] Microsoft Security Advisory CVE-2024-21386: .NET Denial... about 4 hours ago github/codeql-action #2144 Merge releases/v3 into releases/v2 about 4 hours ago github/codeql-action #2143 Mergeback v3.24.2 refs/heads/releases/v3 into main about 5 hours ago github/codeql-action #2142 Merge main into releases/v3 about 5 hours ago github/explore #4236 Bump actions/jekyll-build-pages from 1.0.9 to 1.0.12 dependencies, github_actions about 7 hours ago github/codeql #15622 Java: Expand ExactPathSanitizer to work on the argument of 'equals' too documentation, Java about 5 hours ago github/codeql #15621 C#: Code quality improvements (fixed log message, removed unused interface) C# about 6 hours ago github/docs #31653 Update working-with-the-npm-registry.md triage about 8 hours ago github/advisory-database #3543 [GHSA-xvc9-xwgj-4cq9] Integer Overflow in HeaderMap::reserve() can cause Deni... about 9 hours ago github/advisory-database #3542 [GHSA-xm6r-4466-mr74] OrientDB vulnerable to Improper Privilage Management le... about 10 hours ago ```

That being said, do we 1) change default behavior and/or 2) expand the command help usage to highlight this is how the flag(s) work?

The core maintainers generally hesitate in changing existing behavior, however I would want to do something to highlight this ability to control the filtering better in the existing documentation:

$ gh search prs --help
Search for pull requests on GitHub.

The command supports constructing queries using the GitHub search syntax,
using the parameter and qualifier flags, or a combination of the two.

GitHub search syntax is documented at:
<https://docs.github.com/search-github/searching-on-github/searching-issues-and-pull-requests>

For more information about output formatting flags, see `gh help formatting`.

USAGE
  gh search prs [<query>] [flags]

FLAGS
      --app string              Filter by GitHub App author
      --archived                Restrict search to archived repositories
      --assignee string         Filter by assignee
      --author string           Filter by author
  -B, --base string             Filter on base branch name
      --checks string           Filter based on status of the checks: {pending|success|failure}
      --closed date             Filter on closed at date
      --commenter user          Filter based on comments by user
      --comments number         Filter on number of comments
      --created date            Filter based on created at date
      --draft                   Filter based on draft state
  -H, --head string             Filter on head branch name
      --interactions number     Filter on number of reactions and comments
      --involves user           Filter based on involvement of user
  -q, --jq expression           Filter JSON output using a jq expression
      --json fields             Output JSON with the specified fields
      --label strings           Filter on label
      --language string         Filter based on the coding language
  -L, --limit int               Maximum number of results to fetch (default 30)
      --locked                  Filter on locked conversation status
      --match strings           Restrict search to specific field of issue: {title|body|comments}
      --mentions user           Filter based on user mentions
      --merged                  Filter based on merged state
      --merged-at date          Filter on merged at date
      --milestone title         Filter by milestone title
      --no-assignee             Filter on missing assignee
      --no-label                Filter on missing label
      --no-milestone            Filter on missing milestone
      --no-project              Filter on missing project
      --order string            Order of results returned, ignored unless '--sort' flag is specified: {asc|desc} (default "desc")
      --owner strings           Filter on repository owner
      --project number          Filter on project board number
      --reactions number        Filter on number of reactions
  -R, --repo strings            Filter on repository
      --review string           Filter based on review status: {none|required|approved|changes_requested}
      --review-requested user   Filter on user or team requested to review
      --reviewed-by user        Filter on user who reviewed
      --sort string             Sort fetched results: {comments|reactions|reactions-+1|reactions--1|reactions-smile|reactions-thinking_face|reactions-heart|reactions-tada|interactions|created|updated} (default "best-match")
      --state string            Filter based on state: {open|closed}
      --team-mentions string    Filter based on team mentions
  -t, --template string         Format JSON output using a Go template; see "gh help formatting"
      --updated date            Filter on last updated at date
      --visibility strings      Filter based on repository visibility: {public|private|internal}
  -w, --web                     Open the search query in the web browser

INHERITED FLAGS
  --help   Show help for command

EXAMPLES
  # search pull requests matching set of keywords "fix" and "bug"
  $ gh search prs fix bug

  # search draft pull requests in cli repository
  $ gh search prs --repo=cli/cli --draft

  # search open pull requests requesting your review
  $ gh search prs --review-requested=@me --state=open

  # search merged pull requests assigned to yourself
  $ gh search prs --assignee=@me --merged

  # search pull requests with numerous reactions
  $ gh search prs --reactions=">100"

  # search pull requests without label "bug"
  $ gh search prs -- -label:bug

LEARN MORE
  Use `gh <command> <subcommand> --help` for more information about a command.
  Read the manual at https://cli.github.com/manual

@williammartin : thoughts?

stuart-leitch commented 8 months ago

Thanks so much for an excellent reply @andyfeller. gh search prs --archived=false for the win!

FWIW I scripted up running two queries, one with --archived and excluded the latter results from the former, but --archived=false is much nicer 😀

Personally, I think the --archived=false behaviour should be the default, and the flag should support true|false|both args. But... we are where we are, so improving docs / help may be the best way to help folk in a similar situation.

Suggestion for the docs: --archived Filter based on status of the repository archived status {true|false} EXAMPLE

  # search pull requests only from un-archived repositories (default is all repositories)
  $ gh search prs --owner github --archived=false

I put together a tiny PR for these changes here, thanks again for your help :)

williammartin commented 8 months ago

This UX was so confusing I had to read this thread multiple times and then go actually use the CLI and read the code 😅

I think what happened here was that the archived flag was added to gh search repos pretty early on:

➜ gh search repos --help
...
FLAGS
      --archived                    Filter based on archive state
...
  # search repositories excluding archived repositories
  $ gh search repos --archived=false

And here it kind of makes sense. Like, search for all, unarchived or archived repos seems intuitive.

However, when pulled across to issues and prs this suddenly seems to become a lot less intuitive because it's about searching in repos that have a certain state and the help text hasn't captured that well at all:

➜ gh search issues --help
...
FLAGS
      --app string             Filter by GitHub App author
      --archived               Restrict search to archived repositories
...

As called out we can't change the default behaviour as it would be a breaking change. Strong agreement on updating the docs with additional examples for search issues and search prs and updating the flag help text on search repos with the extra {true|false} business.

stuart-leitch commented 8 months ago

I've made these additional changes to the linked PR (https://github.com/cli/cli/pull/8738). Agree that consistency helps the UX :)

This UX was so confusing I had to read this thread multiple times and then go actually use the CLI and read the code 😅

I was worried it was just me 😆

jsoref commented 2 months ago

Did a good job of fixing the help.

Is this issue left open to contemplate changing the default behavior?

Personally, I agree that searching through archived repositories for open prs is relatively pointless, while they may technically be "open", they aren't practically "open" as the repository is readonly.

If we had a time machine, changing the default to excluding them would be a great idea.

Sadly, gh is used by people who write scripts, so having the behavior change is probably not practical, short of a major version flag day (e.g. the one that replaced hub with gh).

If I worked at github, I'd probably propose:

  1. running a query to see how many gh search prs are run in a month span
  2. and how many of those yield archived prs
  3. and how many of those archived prs are poked w/in a week of being queried (initially I'd limit the entire process to public repositories).
  4. If it turns out that virtually no queries for prs that result in prs in archived repos temporally correspond to inquiries about those prs in archived repos, then I'd make the argument that it'd be worth writing a similar query (but carefully blinded) to ...
  5. find out how many orgs in a similar (or longer) span perform that behavior against private repos initially only yielding (but not any other identifying information):
    • counts of PRs
    • and counts of repos in orgs
    • and counts of orgs
  6. If it turns out that countably few entities actually poke prs in archived repositories after searching for prs, then I'd propose changing the cli's default behavior.