Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect whether action view branch was deleted #32764

Merged
merged 15 commits into from
Dec 12, 2024

Conversation

lunny
Copy link
Member

@lunny lunny commented Dec 8, 2024

Fix #32761

图片

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 8, 2024
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 8, 2024
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/frontend labels Dec 8, 2024
@lunny lunny added this to the 1.23.0 milestone Dec 8, 2024
@yp05327
Copy link
Member

yp05327 commented Dec 9, 2024

Not do it in RefLink()?

In runs_list.tmpl, it is also used. So I guess in runs list page, you will get similar issue.
image

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 10, 2024
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Dec 10, 2024
@lunny
Copy link
Member Author

lunny commented Dec 10, 2024

Not do it in RefLink()?

In runs_list.tmpl, it is also used. So I guess in runs list page, you will get similar issue. image

Done in 890863f

@lunny lunny requested a review from yp05327 December 10, 2024 05:41
@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 Dec 10, 2024
models/git/branch.go Outdated Show resolved Hide resolved
@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 Dec 11, 2024
if refName.IsBranch() {
branchName := refName.ShortName()
run.IsRefDeleted = true // assume it's deleted then if it's found in the database it's not deleted
branchRuns[branchName] = append(branchRuns[branchName], run)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it right? What if a branch starts 2 runs?

Copy link
Contributor

Choose a reason for hiding this comment

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

And since the logic seems fragile, I think it needs tests

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it right? What if a branch starts 2 runs?

A branch could start multiple runs, feature branches maybe updated multiple times and main branches may merge many commits. All these behaviours will trigger action runs.

Copy link
Member Author

Choose a reason for hiding this comment

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

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 12, 2024
@lunny lunny enabled auto-merge (squash) December 12, 2024 18:51
@lunny lunny merged commit 6370d2f into go-gitea:main Dec 12, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 12, 2024
@lunny lunny deleted the lunny/detect_action_branch_deleted branch December 12, 2024 19:29
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. modifies/frontend modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Action view branch button doesn't check for deleted branch
7 participants