Feature detect v1 projects on pr view #10821

williammartin · 2025-04-17T20:15:21Z

Description

Relates to: #10714
Builds on: #10819

This PR tackles projectv1 deprecation on pr view

Reviewer Notes

DO NOT MERGE into base branch, wait for base branch to be merged into trunk.

You can verify the presence or absence of projects by running:

 GH_DEBUG=api ./bin/gh pr view 10820 2>&1 | grep "projectCards"

williammartin · 2025-04-17T20:16:05Z

pkg/cmd/pr/view/view_test.go

@@ -398,6 +402,8 @@ func TestPRView_Preview_nontty(t *testing.T) {

 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
+			t.Cleanup(shared.ResetRunCommandFinder)


These were silly tests that were just leaving global state around that caused bizarre failures:

➜ cli git:(wm/projectsv1-deprecation-pr-view) ✗ go test -count=1 ./pkg/cmd/pr/view --- FAIL: TestProjectsV1Deprecation (0.00s) --- FAIL: TestProjectsV1Deprecation/when_projects_v1_is_supported,_is_included_in_query (0.00s) view_test.go:908: 1 HTTP stubs unmatched, stacks: Stub 1: goroutine 82 [running]: runtime/debug.Stack() /opt/homebrew/Cellar/go/1.24.0/libexec/src/runtime/debug/stack.go:26 +0x64 github.com/cli/cli/v2/pkg/httpmock.(*Registry).Register(...) /Users/williammartin/workspace/cli/pkg/httpmock/registry.go:26 github.com/cli/cli/v2/pkg/cmd/pr/view.TestProjectsV1Deprecation.func1(0x1400009ba40) /Users/williammartin/workspace/cli/pkg/cmd/pr/view/view_test.go:881 +0x88 testing.tRunner(0x1400009ba40, 0x100f52fc0) /opt/homebrew/Cellar/go/1.24.0/libexec/src/testing/testing.go:1792 +0xe4 created by testing.(*T).Run in goroutine 81 /opt/homebrew/Cellar/go/1.24.0/libexec/src/testing/testing.go:1851 +0x374 FAIL FAIL github.com/cli/cli/v2/pkg/cmd/pr/view 0.364s FAIL ➜ cli git:(wm/projectsv1-deprecation-pr-view) ✗ go test -count=1 -run ^TestProjectsV1Deprecation$ ./pkg/cmd/pr/view ok github.com/cli/cli/v2/pkg/cmd/pr/view 0.404s

williammartin · 2025-04-17T20:16:23Z

pkg/cmd/pr/view/view_test.go

@@ -175,6 +176,9 @@ func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*t
 	factory := &cmdutil.Factory{
 		IOStreams: ios,
 		Browser:   browser,
+		HttpClient: func() (*http.Client, error) {
+			return &http.Client{Transport: rt}, nil


Maybe not necessary, but assume this was supposed to be piped through, the variable was unused.

williammartin · 2025-04-17T20:17:55Z

pkg/cmd/pr/shared/finder.go

+			opts.Detector = fd.NewDetector(cachedClient, f.baseRefRepo.RepoHost())
+		}
+
+		if opts.Detector.ProjectsV1() == gh.ProjectsV1Unsupported {


I would have preferred to take the same approach as the issue commands and to have made the decision to add this field in pr view, but I really, really didn't want to untangle the PR Finder like I did with the issues lookup work. Furthermore, the issues work was a lot more viral with a Detector needing to be passed as an arg in many many places, where this is a lot more self contained due to the finder options field being optional.

Did a spike today to explore the other option and it actually ended up adding a lot of extra stuff to pr view that would need to be removed, so think just leaving it as is is best.

williammartin · 2025-04-22T10:20:38Z

I'm moving this back to draft until we've worked through the issue ones.

williammartin · 2025-05-01T16:15:29Z

pkg/cmd/pr/shared/finder.go

@@ -79,6 +80,10 @@ func RunCommandFinder(selector string, pr *api.PullRequest, repo ghrepo.Interfac
 	return finder
 }

+func ResetRunCommandFinder() {


Should probably comment this and understand exactly why it was needed. Frustrating that the old tests were so poorly isolated that we need to investigate for little real value.

I agree. Do you want that in this PR or a follow up?

Yeah this is a bit of a smell, using a package variable for this 🤔

Not looking to untangling this one.

BagToad

LGTM, I don't think anything here is blocking

Ran ATs just in case too:

tests

┌────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│  STATUS │ ELAPSED │                              TEST                              │             PACKAGE               │
│─────────┼─────────┼────────────────────────────────────────────────────────────────┼───────────────────────────────────│
│  PASS   │   17.06 │ TestPullRequests/pr-checkout-with-url-from-fork                │ github.com/cli/cli/v2/acceptance  │
│  PASS   │   16.96 │ TestPullRequests/pr-view-status-respects-branch-pushremote     │ github.com/cli/cli/v2/acceptance  │
│  PASS   │   16.78 │ TestPullRequests/pr-create-guesses-remote-from-sha             │ github.com/cli/cli/v2/acceptance  │
│  PASS   │   16.23 │ TestPullRequests/pr-create-edit-with-project                   │ github.com/cli/cli/v2/acceptance  │
│  PASS   │   15.86 │ TestPullRequests/pr-view-status-respects-remote-pushdefault    │ github.com/cli/cli/v2/acceptance  │
│  PASS   │   14.49 │ TestPullRequests/pr-view-same-org-fork                         │ github.com/cli/cli/v2/acceptance  │
│  PASS   │   11.02 │ TestPullRequests/pr-merge-rebase-strategy                      │ github.com/cli/cli/v2/acceptance  │
│  PASS   │   10.38 │ TestPullRequests/pr-create-from-issue-develop-base             │ github.com/cli/cli/v2/acceptance  │
│  PASS   │   10.32 │ TestPullRequests/pr-comment-new                                │ github.com/cli/cli/v2/acceptance  │
│  PASS   │   10.09 │ TestPullRequests/pr-merge-merge-strategy                       │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    9.27 │ TestPullRequests/pr-comment-edit-last-with-comments            │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    9.17 │ TestPullRequests/pr-create-with-metadata                       │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    8.72 │ TestPullRequests/pr-create-respects-simple-pushdefault         │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    8.48 │ TestPullRequests/pr-create-from-manual-merge-base              │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    8.12 │ TestPullRequests/pr-comment-edit-last-without-comments-creates │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    7.75 │ TestPullRequests/pr-list                                       │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    7.65 │ TestPullRequests/pr-view-status-respects-push-destination      │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    7.56 │ TestPullRequests/pr-create-no-local-repo                       │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    7.53 │ TestPullRequests/pr-view-outside-repo                          │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    7.52 │ TestPullRequests/pr-checkout-by-number                         │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    7.37 │ TestPullRequests/pr-view-status-respects-simple-pushdefault    │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    7.31 │ TestPullRequests/pr-checkout                                   │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    6.74 │ TestPullRequests/pr-view                                       │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    6.64 │ TestPullRequests/pr-create-basic                               │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    6.53 │ TestPullRequests/pr-comment-edit-last-without-comments-errors  │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    6.52 │ TestPullRequests/pr-create-without-upstream-config             │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    0.00 │ TestPullRequests                                               │ github.com/cli/cli/v2/acceptance  │
│  SKIP   │    0.00 │ TestPullRequests/pr-create-respects-branch-pushremote          │ github.com/cli/cli/v2/acceptance  │
│  SKIP   │    0.00 │ TestPullRequests/pr-create-respects-push-destination           │ github.com/cli/cli/v2/acceptance  │
│  SKIP   │    0.00 │ TestPullRequests/pr-create-respects-remote-pushdefault         │ github.com/cli/cli/v2/acceptance  │
│  SKIP   │    0.00 │ TestPullRequests/pr-create-respects-user-colon-branch-syntax   │ github.com/cli/cli/v2/acceptance  │
│  SKIP   │    0.00 │ TestPullRequests/pr-status-respects-cross-org                  │ github.com/cli/cli/v2/acceptance  │
└────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘
┌────────────────────────────────────────────────────────────────────────────────────┐
│  STATUS │ ELAPSED │             PACKAGE              │ COVER │ PASS │ FAIL │ SKIP  │
│─────────┼─────────┼──────────────────────────────────┼───────┼──────┼──────┼───────│
│  PASS   │ 36.03s  │ github.com/cli/cli/v2/acceptance │  --   │  27  │  0   │  5    │
└────────────────────────────────────────────────────────────────────────────────────┘

andyfeller

Fantastic work making the PR finder testing logic understandable for future maintainership

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.72.0` -> `v2.73.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.73.0`](https://github.com/cli/cli/releases/tag/v2.73.0): GitHub CLI 2.73.0 [Compare Source](cli/cli@v2.72.0...v2.73.0) ####

Copilot Coding Agent Support You can now assign issues to GitHub Copilot directly from `gh`, just as you would assign them to a teammate. Use `gh issue edit <number> --add-assignee @copilot` to assign the GitHub Copilot coding agent, and Copilot will work in the background to understand the issue, propose a solution, and open a pull request when it's ready for your review. If you run `gh issue edit` interactively, `Copilot (AI)` will be displayed as a potential assignee. This feature is available for GitHub Copilot Pro+ and Copilot Enterprise subscribers. For more details, refer to [the full changelog post for Copilot coding agent](https://github.blog/changelog/2025-05-19-github-copilot-coding-agent-in-public-preview/). #### What's Changed ##### ✨ Features - Copilot is assignable to issues and pull requests with `issue edit` and `pr edit` by [@BagToad](https://github.com/BagToad) in cli/cli#10992 - `gh issue edit`: actors are assignable to issues by [@BagToad](https://github.com/BagToad) in cli/cli#10960 - `gh pr edit`: Assign actors to pull requests by [@BagToad](https://github.com/BagToad) in cli/cli#10984 - `issue edit`, `pr edit`: handle display names in interactive assignee editing by [@BagToad](https://github.com/BagToad) in cli/cli#10990 - `issue edit`, `pr edit`: Support special non-interactive (flags) assignee name `@copilot` by [@BagToad](https://github.com/BagToad) in cli/cli#10991 - \[gh issue/pr comment] Add support for last comment delete for issues and MRs by [@sinansonmez](https://github.com/sinansonmez) in cli/cli#10596 - \[gh issue view] Expose `closedByPullRequestsReferences` JSON field by [@iamazeem](https://github.com/iamazeem) in cli/cli#10941 - Accessible prompter always displays selection defaults in a format readable by a screen reader by [@BagToad](https://github.com/BagToad) in cli/cli#10937 ##### 🐛 Fixes - Fix `StatusJSONResponse` usage by [@babakks](https://github.com/babakks) in cli/cli#10810 - Fix panic on `gh pr view 0` by [@nopcoder](https://github.com/nopcoder) in cli/cli#10729 - Fix flakey test for accessible prompter by [@BagToad](https://github.com/BagToad) in cli/cli#10918 - Fix accessible prompter flaky tests by [@babakks](https://github.com/babakks) in cli/cli#10977 - Handle missing archive URLs on release download by [@williammartin](https://github.com/williammartin) in cli/cli#10947 - Fix bug when removing all MR reviewers by [@babakks](https://github.com/babakks) in cli/cli#10975 ##### 📚 Docs & Chores - Feature detect v1 projects on pr view by [@williammartin](https://github.com/williammartin) in cli/cli#10821 - Feature detect v1 projects on non-interactive pr create by [@williammartin](https://github.com/williammartin) in cli/cli#10909 - Feature detect v1 projects on web mode pr create by [@williammartin](https://github.com/williammartin) in cli/cli#10911 - Feature detect v1 projects on interactive `pr create` by [@williammartin](https://github.com/williammartin) in cli/cli#10915 - Feature detect v1 projects on pr edit by [@williammartin](https://github.com/williammartin) in cli/cli#10942 - Move predicate type filtering in `gh attestation verify` by [@malancas](https://github.com/malancas) in cli/cli#10670 - Improve assertion for disabled echo mode by [@babakks](https://github.com/babakks) in cli/cli#10927 #####

Dependencies - chore(deps): bump actions/attest-build-provenance from 2.2.2 to 2.3.0 by [@dependabot](https://github.com/dependabot) in cli/cli#10886 - chore(deps): bump github.com/cpuguy83/go-md2man/v2 from 2.0.6 to 2.0.7 by [@dependabot](https://github.com/dependabot) in cli/cli#10869 #### What's Changed #### New Contributors - [@sinansonmez](https://github.com/sinansonmez) made their first contribution in cli/cli#10596 - [@nopcoder](https://github.com/nopcoder) made their first contribution in cli/cli#10729 **Full Changelog**: cli/cli@v2.72.0...v2.73.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

williammartin requested a review from a team as a code owner April 17, 2025 20:15

williammartin requested review from BagToad and removed request for a team April 17, 2025 20:15

williammartin temporarily deployed to cli-automation April 17, 2025 20:15 — with GitHub Actions Inactive

cliAutomation assigned williammartin Apr 17, 2025

williammartin commented Apr 17, 2025

View reviewed changes

williammartin marked this pull request as draft April 22, 2025 10:20

Base automatically changed from wm/projectsv1-deprecation-issue-edit to trunk April 23, 2025 12:18

Feature detect v1 projects on pr view

3bcf975

williammartin force-pushed the wm/projectsv1-deprecation-pr-view branch from 000d7ee to 3bcf975 Compare May 1, 2025 13:31

williammartin marked this pull request as ready for review May 1, 2025 14:01

williammartin temporarily deployed to cli-automation May 1, 2025 14:01 — with GitHub Actions Inactive

williammartin mentioned this pull request May 1, 2025

Feature detect v1 projects on non-interactive pr create #10909

Merged

williammartin commented May 1, 2025

View reviewed changes

BagToad approved these changes May 1, 2025

View reviewed changes

Cleanup run command stubbed finders in tests

64370ce

williammartin force-pushed the wm/projectsv1-deprecation-pr-view branch from 81354c1 to 64370ce Compare May 2, 2025 12:48

andyfeller approved these changes May 2, 2025

View reviewed changes

williammartin merged commit c5d8e6f into trunk May 2, 2025
16 checks passed

williammartin deleted the wm/projectsv1-deprecation-pr-view branch May 2, 2025 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature detect v1 projects on pr view #10821

Feature detect v1 projects on pr view #10821

Uh oh!

williammartin commented Apr 17, 2025

Uh oh!

williammartin Apr 17, 2025

Uh oh!

williammartin Apr 17, 2025

Uh oh!

williammartin Apr 17, 2025

Uh oh!

williammartin May 1, 2025

Uh oh!

williammartin commented Apr 22, 2025

Uh oh!

williammartin May 1, 2025

Uh oh!

BagToad May 1, 2025

Uh oh!

andyfeller May 1, 2025

Uh oh!

BagToad left a comment •

edited

Loading

Uh oh!

andyfeller left a comment

Uh oh!

Uh oh!

Uh oh!

Feature detect v1 projects on pr view #10821

Feature detect v1 projects on pr view #10821

Uh oh!

Conversation

williammartin commented Apr 17, 2025

Description

Reviewer Notes

Uh oh!

williammartin Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

williammartin Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

williammartin Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

williammartin May 1, 2025

Choose a reason for hiding this comment

Uh oh!

williammartin commented Apr 22, 2025

Uh oh!

williammartin May 1, 2025

Choose a reason for hiding this comment

Uh oh!

BagToad May 1, 2025

Choose a reason for hiding this comment

Uh oh!

andyfeller May 1, 2025

Choose a reason for hiding this comment

Uh oh!

BagToad left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andyfeller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BagToad left a comment •

edited

Loading