Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Also display "recently pushed branch" alert on PR view#35001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
wxiaoguang merged 5 commits intogo-gitea:mainfromNaxdy:work/pr-on-pr-view
Jul 10, 2025

Conversation

@Naxdy
Copy link
Contributor

@NaxdyNaxdy commentedJul 8, 2025•
edited
Loading

This commit adds the "You recently pushed to branch X" alert also to PR overview, as opposed to only the repository's home page.

GitHub also shows this alert on the PR list, as well as the home page, which makes sense, since a common flow is:

  1. Go to issues page
  2. Grab an issue to work on
  3. Branch off
  4. Work
  5. Push
  6. Open PR

For step 6, many devs' intuition is to go directly to the pull request page, to open a new PR. However, in current Gitea, this is actually not optimal, since this would require you to enter your PR details manually, and going to the home page would let you take a shortcut, which is unintuitive.

Screenshots:

PR List:

image

anbraten and wxiaoguang reacted with thumbs up emoji
@GiteaBotGiteaBot added the lgtm/need 2This PR needs two approvals by maintainers to be considered for merging. labelJul 8, 2025
@github-actionsgithub-actionsbot added modifies/goPull requests that update Go code modifies/templatesThis PR modifies the template files labelsJul 8, 2025
@lunnylunny added the type/enhancementAn improvement of existing functionality labelJul 8, 2025
@lunnylunny added this to the1.25.0 milestoneJul 8, 2025
@silverwind
Copy link
Member

But GH does not show it on the issues page right? Why should we?

@Naxdy
Copy link
ContributorAuthor

My thought was mainly because of time tracking. So if you work on something, then go to stop your time tracking, you have the alert close by. I have no strong feelings towards that part however.

@GiteaBotGiteaBot added lgtm/need 1This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2This PR needs two approvals by maintainers to be considered for merging. labelsJul 8, 2025
@silverwind
Copy link
Member

Maybe keep it to the PR page. Issue page with issue pins visible would be quite overloaded imho.

@wxiaoguang
Copy link
Contributor

Two things in my mind:

  1. Please keep the function in "route" related packages
  2. Please don't show the banner on the "issue list" page

@NaxdyNaxdy changed the titleDisplay "new pull request" alert also on issue & PR listsAlso display "recently pushed branch" alert on PR viewJul 9, 2025
@Naxdy
Copy link
ContributorAuthor

Changed it to only show on the PR page due to popular demand, and also moved the file back torouters/utils.

anbraten, delvh, and silverwind reacted with thumbs up emoji

@NaxdyNaxdy requested a review fromlunnyJuly 9, 2025 09:09
@silverwind
Copy link
Member

silverwind commentedJul 9, 2025•
edited
Loading

We can show it on the repo homepage though, example from GitHub:

image

BTW I wish there was a way to never show these messages for certain branches, like ourrelease/v1.24 which is a release branch and not meant to be pulled into main.

@Naxdy
Copy link
ContributorAuthor

We can show it on the repo homepage though, example from GitHub:

This is the current behavior, which I didn't change. Sorry if I made it seem like I removed that functionality in favor of having itonly on the PR view, that's not the case! :)

@silverwind
Copy link
Member

silverwind commentedJul 9, 2025•
edited
Loading

Ah I wasn't aware. So we show it on:

  • Frontpage
  • Repo Homepage
  • Pull Requests List (with this PR)

Is that correct?

@Naxdy
Copy link
ContributorAuthor

Yep, that covers it!

@GiteaBotGiteaBot added lgtm/doneThis PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1This PR needs approval from one additional maintainer to be merged. labelsJul 9, 2025
@wxiaoguangwxiaoguang marked this pull request as draftJuly 10, 2025 01:04
@wxiaoguang
Copy link
Contributor

wxiaoguang commentedJul 10, 2025•
edited
Loading

Thank you very much for the PR. Your change is good enough. I made some more changes, some ideas are:

  • Move the code from "route/utils" package to "route/repo" package
    • By reading other code in "route/utils" package, it seems that "util" is more suitable for some functions to "help"
    • Our prepareRecentlyPushedNewBranches prepares the template rendering data, so "route/repo" seems more suitable
  • Introducerepo.CanContentChange to replaceIsMirror check
    • That's a more complete check for whether a repo is changeable
    • I found the problem when working on other PRs, now it seems a good chance to introduce it and make prepareRecentlyPushedNewBranches better
  • Introduce a dedicated struct to pass "RecentBranchesPromptData"
    • Golang template is quite fragile, so if there is a chance, I'd like to clearly define the variable name and struct, to make it less fragile.
    • And no need to use theif PageIsPull check, now the template only renders when there is "RecentBranchesPromptData"
  • Fine tune the template and simplify thetw-xx helpers

So I made some changes that I'd like to do sooner or later in this PR together 😆 🙏

Screenshot (no visual change):

Details

image

image

image

@wxiaoguangwxiaoguang marked this pull request as ready for reviewJuly 10, 2025 01:47
@lunnylunny added the reviewed/wait-mergeThis pull request is part of the merge queue. It will be merged soon. labelJul 10, 2025
@wxiaoguangwxiaoguangenabled auto-merge (squash)July 10, 2025 16:45
@wxiaoguangwxiaoguang merged commit32152a0 intogo-gitea:mainJul 10, 2025
26 checks passed
@GiteaBotGiteaBot removed the reviewed/wait-mergeThis pull request is part of the merge queue. It will be merged soon. labelJul 10, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull requestJul 11, 2025
* giteaofficial/main:  Fix updating user visibility (go-gitea#35036)  Fix git commit committer parsing and add some tests (go-gitea#35007)  Refactor OpenIDConnect to support SSH/FullName sync (go-gitea#34978)  Support base64-encoded agit push options (go-gitea#35037)  Also display "recently pushed branch" alert on PR view (go-gitea#35001)  Make submodule link work with relative path (go-gitea#35034)  Update to go 1.24.5 (go-gitea#35031)  Improve CLI commands (go-gitea#34973)  Tweak eslint config, fix new issues (go-gitea#35019)# Conflicts:#templates/repo/commits_list.tmpl
@go-giteago-gitea locked asresolvedand limited conversation to collaboratorsOct 9, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@delvhdelvhdelvh approved these changes

@lunnylunnylunny approved these changes

@wxiaoguangwxiaoguangwxiaoguang approved these changes

Assignees

No one assigned

Labels

lgtm/doneThis PR has enough approvals to get merged. There are no important open reservations anymore.modifies/goPull requests that update Go codemodifies/templatesThis PR modifies the template filestype/enhancementAn improvement of existing functionality

Projects

None yet

Milestone

1.25.0

Development

Successfully merging this pull request may close these issues.

6 participants

@Naxdy@silverwind@wxiaoguang@lunny@delvh@GiteaBot
[8]ページ先頭
©2009-2025 Movatter.jp