8000 fix double 'push tag' action feed (#15078) by a1012112796 · Pull Request #15083 · go-gitea/gitea · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix double 'push tag' action feed (#15078) #15083

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
Merged
merged 1 commit into from
Mar 21, 2021
Merged

fix double 'push tag' action feed (#15078) #15083

merged 1 commit into from
Mar 21, 2021

Conversation

a1012112796
Copy link
Member

Backport #15078

@a1012112796
fix double 'push tag' action feed (go-gitea#15078)
6cbc7ac 
Signed-off-by: a1012112796 <1012112796@qq.com>
@a1012112796 a1012112796 added this to the 1.14.0 milestone Mar 21, 2021
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 21, 2021
@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 21, 2021
@zeripath zeripath merged commit fbaa019 into go-gitea:release/v1.14 Mar 21, 2021
@a1012112796 a1012112796 deleted the backport/15078 branch March 21, 2021 14:52
@techknowlogick
Copy link
Member

Does this affect webhooks? For example with Drone?

@lunny
Copy link
Member
lunny commented Mar 22, 2021

Imo it will. The PR should be reverted. We need to change the behaviour of push commits but not remove push tag.

@a1012112796
Copy link
Member Author

here ....

gitea/modules/notification/webhook/webhook.go

Lines 563 to 583 in a587a28

func (m *webhookNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) {
apiPusher := convert.ToUser(pusher, false, false)
apiCommits, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
if err != nil {
log.Error("commits.ToAPIPayloadCommits failed: %v", err)
return
}
if err := webhook_services.PrepareWebhooks(repo, models.HookEventPush, &api.PushPayload{
Ref: opts.RefFullName,
Before: opts.OldCommitID,
After: opts.NewCommitID,
CompareURL: setting.AppURL + commits.CompareURL,
Commits: apiCommits,
Repo: convert.ToRepo(repo, models.AccessModeOwner),
Pusher: apiPusher,
Sender: apiPusher,
}); err != nil {
log.Error("PrepareWebhooks: %v", err)
}
}

@silverwind
Copy link
Member
silverwind commented Mar 22, 2021 •
edited
Loading

Hope this won't introduce a regression in the tag webhooks sent to drone and the like, like the one fixed in #13999. Before the fix, tag pushes were not progagated to drone leading to triggers like this one not being executed:

trigger:
  event:
    - tag

@a1012112796
Copy link
Member Author

@techknowlogick @lunny @silverwind test result on master branch (1.15.0+dev-14-g1a03fa7a4)
tmp

@silverwind
Copy link
Member

Ok, if the second hook there has ref_type: tag, it should be fine.

@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@silverwind silverwind silverwind approved these changes

@zeripath zeripath zeripath approved these changes

@6543 6543 6543 approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Milestone
1.14.0
Development

Successfully merging this pull request may close these issues.
7 participants
@a1012112796 @techknowlogick @lunny @silverwind @zeripath @6543 @GiteaBot
0