Refactor pull request view (1) by wxiaoguang · Pull Request #37380 · go-gitea/gitea

wxiaoguang · 2026-04-23T07:11:21Z

Need to do step by step

Refactor preparePullViewPullInfo and related functions, split them into small ones:

preparePullViewPullInfo creates PullRequestViewInfo struct
if the PR is merged: prepareViewMergedPullInfo
if the PR is open: prepareViewOpenPullInfo

In prepareViewMergedPullInfo and preparePullViewFillInfo: call preparePullViewFillInfo consistnently

preparePullViewFillInfo calls preparePullViewFillCompareInfo and preparePullViewFillCommitStatusInfo

Copilot

Pull request overview

This PR continues an incremental refactor of the pull request view by introducing a structured PullRequestViewInfo, centralizing compare-info preparation, and switching GetCompareInfo to return a value to better support partial results.

Changes:

Refactor git_service.GetCompareInfo to return a CompareInfo value (instead of a pointer) and populate commit IDs up-front (joining resolution errors).
Introduce PullRequestViewInfo and consolidate PR compare-info filling logic in routers/web/repo/pull.go.
Add gitcmd.StderrContains helper and update compare-info parsing helpers to return pointers to local CompareInfo values.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
services/git/compare.go	Changes `GetCompareInfo` to return `CompareInfo` by value and partially populate fields earlier.
routers/web/repo/pull.go	Adds `PullRequestViewInfo` and refactors PR view/compare preparation logic around it.
routers/web/repo/issue_view.go	Runs PR-specific prepare functions only for pull issues.
routers/web/repo/compare.go	Updates `ParseCompareInfo` to return `&compareInfo` after `GetCompareInfo` becomes value-returning.
routers/api/v1/repo/pull.go	Adjusts API compare parsing and PR commits/files endpoints for value-returning `GetCompareInfo`.
modules/git/gitcmd/error.go	Adds `StderrContains` helper for stderr-based error matching.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

silverwind · 2026-04-23T22:58:07Z

One suggestion on the new helpers: preparePullViewFillCompareInfo, preparePullViewFillCommitStatusInfo, and preparePullViewFillCommitStatusInfoForOpen each re-fetch the struct via ctx.Data["PullRequestViewInfo"].(*PullRequestViewInfo).

Since they're only reachable through preparePullViewPullInfo, passing prInfo *PullRequestViewInfo as an explicit parameter would remove the type-assertion panic-on-misuse and matches the stated direction in the PullRequestViewInfo TODO comment (decoupling from template data).

Reviewed with the help of Claude Opus 4.7

wxiaoguang · 2026-04-24T05:28:33Z

One suggestion on the new helpers: preparePullViewFillCompareInfo, preparePullViewFillCommitStatusInfo, and preparePullViewFillCommitStatusInfoForOpen each re-fetch the struct via ctx.Data["PullRequestViewInfo"].(*PullRequestViewInfo).

Since they're only reachable through preparePullViewPullInfo, passing prInfo *PullRequestViewInfo as an explicit parameter would remove the type-assertion panic-on-misuse and matches the stated direction in the PullRequestViewInfo TODO comment (decoupling from template data).

Reviewed with the help of Claude Opus 4.7

silverwind

Context abuse is ok to fix later.

BTW I would prefer it we get rid of all functions in Context. Context is for data, not functions. Functions should be template helpers.

* origin/main: (127 commits) Refactor pull request view (1) (go-gitea#37380) Improve AGENTS.md (go-gitea#37382) Remove dead CSS (go-gitea#37376) Add pr-review e2e test and speed up e2e tests (go-gitea#37345) Drop Fomantic tab, checkbox and form patches (go-gitea#37377) fix: dump with default zip type produces uncompressed zip (go-gitea#37401) Allow fast-forward-only merge when signed commits are required (go-gitea#37335) Introduce `ActionRunAttempt` to represent each execution of a run (go-gitea#37119) Move review request functions to a standalone file (go-gitea#37358) Fix repo init README EOL (go-gitea#37388) Fix org team assignee/reviewer lookups for team member permissions (go-gitea#37365) Remove external service dependencies in migration tests (go-gitea#36866) Extend issue context popup beyond markdown content (go-gitea#36908) fix: commit status reporting (go-gitea#37372) Support for Custom URI Schemes in OAuth2 Redirect URIs (go-gitea#37356) Fix cmd tests by mocking builtin paths (go-gitea#37369) chore: upgrade Go version in devcontainer image to 1.26 (go-gitea#37374) Fix button layout shift when collapsing file tree in editor (go-gitea#37363) Update `Block a user` form (go-gitea#37359) Remove IsValidExternalURL/IsAPIURL and use IsValidURL at call sites (go-gitea#37364) ... # Conflicts: # modules/eventsource/event.go # tests/e2e/events.test.ts

* origin/main: feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354) Refactor pull request view (1) (go-gitea#37380) Improve AGENTS.md (go-gitea#37382) Remove dead CSS (go-gitea#37376) Add pr-review e2e test and speed up e2e tests (go-gitea#37345) Drop Fomantic tab, checkbox and form patches (go-gitea#37377)

* origin/main: (51 commits) Fix color regressions, add `priority` color (go-gitea#37417) [skip ci] Updated translations via Crowdin Stabilize e2e logout propagation test (go-gitea#37403) refactor: serve site manifest via `/assets/site-manifest.json` endpoint (go-gitea#37405) feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354) Refactor pull request view (1) (go-gitea#37380) Improve AGENTS.md (go-gitea#37382) Remove dead CSS (go-gitea#37376) Add pr-review e2e test and speed up e2e tests (go-gitea#37345) Drop Fomantic tab, checkbox and form patches (go-gitea#37377) fix: dump with default zip type produces uncompressed zip (go-gitea#37401) Allow fast-forward-only merge when signed commits are required (go-gitea#37335) Introduce `ActionRunAttempt` to represent each execution of a run (go-gitea#37119) Move review request functions to a standalone file (go-gitea#37358) Fix repo init README EOL (go-gitea#37388) Fix org team assignee/reviewer lookups for team member permissions (go-gitea#37365) Remove external service dependencies in migration tests (go-gitea#36866) Extend issue context popup beyond markdown content (go-gitea#36908) fix: commit status reporting (go-gitea#37372) Support for Custom URI Schemes in OAuth2 Redirect URIs (go-gitea#37356) ...

Follow up #37380 Some code is moved to the place whether it should be.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 23, 2026

wxiaoguang mentioned this pull request Apr 23, 2026

Allow fast-forward-only merge when signed commits are required #37335

Merged

wxiaoguang force-pushed the refactor-pr-view branch from 0823a9a to 54daf12 Compare April 23, 2026 07:17

wxiaoguang changed the title ~~Refactor pull reuqest view (1)~~ Refactor pull request view (1) Apr 23, 2026

wxiaoguang force-pushed the refactor-pr-view branch 3 times, most recently from 72167c2 to a07cac4 Compare April 23, 2026 08:11

wxiaoguang requested a review from Copilot April 23, 2026 08:55

Copilot started reviewing on behalf of wxiaoguang April 23, 2026 08:56 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread routers/web/repo/pull.go Outdated

Comment thread routers/web/repo/pull.go Outdated

Comment thread routers/web/repo/pull.go Outdated

wxiaoguang force-pushed the refactor-pr-view branch 3 times, most recently from b794e69 to b5919b6 Compare April 23, 2026 09:48

wxiaoguang requested a review from Copilot April 23, 2026 09:48

Copilot started reviewing on behalf of wxiaoguang April 23, 2026 09:49 View session

wxiaoguang force-pushed the refactor-pr-view branch 2 times, most recently from 5b6bbc6 to 7cd4fcf Compare April 23, 2026 09:52

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread routers/web/repo/pull.go

Comment thread routers/web/repo/pull.go

Comment thread modules/git/gitcmd/error.go

wxiaoguang force-pushed the refactor-pr-view branch from 7cd4fcf to f6b192c Compare April 23, 2026 09:58

wxiaoguang requested a review from Copilot April 23, 2026 09:58

Copilot started reviewing on behalf of wxiaoguang April 23, 2026 09:58 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread routers/web/repo/pull.go Outdated

Comment thread services/git/compare.go Outdated

Comment thread routers/web/repo/pull.go

wxiaoguang force-pushed the refactor-pr-view branch 2 times, most recently from 4a084f1 to db27ec6 Compare April 23, 2026 10:35

fix

1701824

wxiaoguang force-pushed the refactor-pr-view branch from db27ec6 to 1701824 Compare April 23, 2026 10:42

wxiaoguang requested a review from Copilot April 23, 2026 10:45

Copilot started reviewing on behalf of wxiaoguang April 23, 2026 10:50 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

silverwind added type/refactoring Existing code has been cleaned up. There should be no new functionality. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Apr 23, 2026

lunny approved these changes Apr 24, 2026

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 24, 2026

lunny added this to the 1.27.0 milestone Apr 24, 2026

wxiaoguang added 2 commits April 24, 2026 17:38

Merge branch 'main' into refactor-pr-view

87bcbdc

Merge branch 'main' into refactor-pr-view

3cf7b9f

silverwind approved these changes Apr 24, 2026

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 24, 2026

Merge branch 'main' into refactor-pr-view

7a0c511

wxiaoguang enabled auto-merge (squash) April 24, 2026 09:53

wxiaoguang merged commit 1483291 into go-gitea:main Apr 24, 2026
26 checks passed

wxiaoguang deleted the refactor-pr-view branch April 26, 2026 07:28

wxiaoguang mentioned this pull request Apr 26, 2026

Refactor pull request view (2) #37428

Merged

wxiaoguang added a commit that referenced this pull request Apr 26, 2026

Refactor pull request view (2) (#37428)

b3ed4cd

Follow up #37380 Some code is moved to the place whether it should be.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor pull request view (1)#37380

Refactor pull request view (1)#37380
wxiaoguang merged 4 commits intogo-gitea:mainfrom
wxiaoguang:refactor-pr-view

wxiaoguang commented Apr 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

silverwind commented Apr 23, 2026 •

edited

Loading

Uh oh!

wxiaoguang commented Apr 24, 2026

Uh oh!

silverwind left a comment •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

wxiaoguang commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

silverwind commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Apr 24, 2026

Uh oh!

silverwind 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wxiaoguang commented Apr 23, 2026 •

edited

Loading

silverwind commented Apr 23, 2026 •

edited

Loading

silverwind left a comment •

edited

Loading