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.

Sign up for GitHub

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

Jump to bottom

Fix invalid link of the commit status when ref is tag #29752

Merged
merged 6 commits into from
Mar 19, 2024
Merged

Fix invalid link of the commit status when ref is tag #29752

merged 6 commits into from
Mar 19, 2024

Conversation

yp05327
Copy link
Contributor

@yp05327 yp05327 commented Mar 13, 2024

Fix #29731
Caused by #24634
Also remove fixme.

ps: we can not fix the existed runs, as wrong refs are all recorded in DB, and we can not know whether they are branch or tag:
image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 13, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 13, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 13, 2024
@yp05327 yp05327 added type/bug modifies/go Pull requests that update Go code and removed modifies/go Pull requests that update Go code labels Mar 13, 2024
@lunny lunny added the backport/v1.21 This PR should be backported to Gitea 1.21 label Mar 13, 2024
@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 Mar 13, 2024
@lunny
Copy link
Member

lunny commented Mar 13, 2024

It's better to have a test.

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 13, 2024

How to test notifier? 🤔

@lunny
Copy link
Member

lunny commented Mar 13, 2024

How to test notifier? 🤔

We can trigger the event manually and check the database.

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 13, 2024

It seems that we have no action integrate tests. Maybe this is a good start.

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 13, 2024
@yp05327
Copy link
Contributor Author

yp05327 commented Mar 13, 2024 •
edited
Loading

So lucky, I would say, by the test I found another issue:
delete tag will remove the ref first , then trigger a notify, then you will got this error
image

So wait a minute, and let me fix it also.

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 13, 2024 •
edited
Loading

delete tag will trigger 3 events:

  • push event, refs/tags/deleted
    push commit ID: 0000000000000000000000000000000000000000
    This one maybe should be ignored?
  • delete event, empty ref
    no ref provided, so default branch will be used
  • release event, deleted action, refs/tags/deleted
    This one seems should be triggered, but don't work now

So for the delete event, the expected ref is default branch, not the deleted tag?
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#delete

@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 Mar 14, 2024
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 14, 2024
@yp05327
Copy link
Contributor Author

yp05327 commented Mar 19, 2024

Tests are simple now.
And you can see the error mentioned above in tests locally:
image

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 19, 2024

Tested in GitHub, delete a branch will only trigger delete event:
https://github.com/yp05327/test2/actions/workflows/testdelete.yml
https://github.com/yp05327/test2/blob/main/.github/workflows/testdelete.ym

@yp05327 yp05327 requested review from lunny and wolfogre March 19, 2024 06:05
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Mar 19, 2024
@GiteaBot GiteaBot removed the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 19, 2024
@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 Mar 19, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 19, 2024
@lunny lunny enabled auto-merge (squash) March 19, 2024 06:56
@lunny lunny merged commit 98217b0 into go-gitea:main Mar 19, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Mar 19, 2024
@GiteaBot
Copy link
Contributor

I was unable to create a backport for 1.21. @yp05327, please send one manually. 🍵

go run ./contrib/backport 29752
...  // fix git conflicts if any
go run ./contrib/backport --continue

@GiteaBot GiteaBot added backport/manual No power to the bots! Create your backport yourself! and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Mar 19, 2024
@yp05327 yp05327 deleted the fix-29731 branch March 19, 2024 08:40
@yp05327 yp05327 mentioned this pull request Mar 19, 2024
Merged
@lunny lunny added the backport/done All backports for this PR have been created label Mar 19, 2024
lunny added a commit that referenced this pull request Mar 20, 2024
@yp05327 @lunny
Fix invalid link of the commit status when ref is tag (#29752) (#29908)
aae96cc 
Backport #29752

---------

Co-authored-by: Lunny Xiao <[email protected]>
@lunny lunny modified the milestones: 1.23.0, 1.22.0 Mar 22, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@wolfogre wolfogre wolfogre approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/bug
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.

Gitea actions reference invalid link for Git tag
4 participants
@yp05327 @lunny @GiteaBot @wolfogre