Skip to content

Refactor pull request view (4)#37451

Merged
silverwind merged 4 commits intogo-gitea:mainfrom
wxiaoguang:refactor-pr-view-4
Apr 28, 2026
Merged

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

Conversation

@wxiaoguang
Copy link
Copy Markdown
Contributor

Use JSON attribute instead of inline script

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 27, 2026
@wxiaoguang wxiaoguang requested a review from Copilot April 27, 2026 10:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the pull request merge box to stop relying on inline <script type="module"> for populating merge-form data, and instead passes the merge form’s initial state via a JSON-encoded HTML attribute into the Vue component.

Changes:

  • Replace inline script-based merge-form data injection with a data-merge-form-props JSON attribute consumed by PullRequestMergeForm.vue.
  • Move merge-form props construction into a dedicated backend helper (prepareMergeBoxFormProps) and store it in PullMergeBoxData.
  • Adjust templates to use the new merge-box data structure for conditional rendering of pull command instructions.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
web_src/js/features/repo-issue-pull.ts Parses merge-form props JSON from DOM and mounts Vue component; removes dynamic script execution on merge-box reload.
web_src/js/components/PullRequestMergeForm.vue Switches merge-form data source from window.config.pageData to passed-in Vue props.
templates/repo/issue/view_content/pull_merge_instruction.tmpl Reads ShowMergeInstructions via MergeBoxData instead of a direct template variable.
templates/repo/issue/view_content/pull_merge_box.tmpl Replaces inline script with data-merge-form-props attribute and updates instruction template invocation.
routers/web/repo/pull_merge_form.go New backend helper to build the merge form props payload and attach it to merge-box data.
routers/web/repo/pull.go Refactors head-target display computation into pullRequestViewInfo while keeping template data intact.
routers/web/repo/issue_view.go Moves merge-form related data preparation into PullMergeBoxData and invokes the new merge-form props builder.

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

Comment thread templates/repo/issue/view_content/pull_merge_box.tmpl
Comment thread routers/web/repo/pull_merge_form.go
Comment thread web_src/js/features/repo-issue-pull.ts
Comment thread web_src/js/features/repo-issue-pull.ts
Comment thread routers/web/repo/pull_merge_form.go Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.


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

Comment thread web_src/js/components/PullRequestMergeForm.vue
@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Apr 27, 2026
@GiteaBot 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 27, 2026
@GiteaBot 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 27, 2026
@silverwind silverwind added reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Apr 27, 2026
Comment thread routers/web/repo/pull_merge_form.go Outdated
@silverwind
Copy link
Copy Markdown
Member

#37451 (comment) seems like a real regression, recommend not merging until fixed.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Apr 28, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Apr 28, 2026

#37451 (comment) seems like a real regression, recommend not merging until fixed.

Typo, fixed in 0a5ee25

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Apr 28, 2026
@silverwind silverwind enabled auto-merge (squash) April 28, 2026 04:14
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 28, 2026
@silverwind silverwind merged commit 8bf51da into go-gitea:main Apr 28, 2026
26 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 28, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 28, 2026
@wxiaoguang wxiaoguang deleted the refactor-pr-view-4 branch April 28, 2026 04:39
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 29, 2026
* main:
  Add DEFAULT_TITLE_SOURCE setting for pull request title default behavior (go-gitea#37465)
  Fix compare dropdown for branches without common history (go-gitea#37470)
  FIX: URL sanitization to handle schemeless credentials (go-gitea#37440)
  Refactor pull request view (4) (go-gitea#37451)
  Fix scheduled action panic with null event payload (go-gitea#37459)
  Fix attachment Content-Security-Policy (go-gitea#37455)
  [skip ci] Updated translations via Crowdin
  Rename CurrentRefPath to CurrentRefSubURL (go-gitea#37453)
  Clean up org pages layout (go-gitea#37445)
  Fix script error alert (go-gitea#37458)
  Fix inconsistent disabled styling on logged-out repo header buttons (go-gitea#37406)
  Add API endpoint to reply to pull request review comments (go-gitea#36683)
  Add CurrentURL template variable back (go-gitea#37444)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants