-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Move labels webhooks to notification #8749
Conversation
services/issue/label.go
Outdated
@@ -84,7 +36,7 @@ func AddLabels(issue *models.Issue, doer *models.User, labels []*models.Label) e | |||
return err | |||
} | |||
|
|||
sendLabelUpdatedWebhook(issue, doer) | |||
notification.NotifyIssueChangeLabels(doer, issue, nil, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
notification.NotifyIssueChangeLabels(doer, issue, nil, nil) | |
notification.NotifyIssueChangeLabels(doer, issue, labels, nil) |
why not use the new parameter - we can implement the handling at NotifyIssueChangeLabels later?
services/issue/label.go
Outdated
@@ -74,7 +26,7 @@ func AddLabel(issue *models.Issue, doer *models.User, label *models.Label) error | |||
return err | |||
} | |||
|
|||
sendLabelUpdatedWebhook(issue, doer) | |||
notification.NotifyIssueChangeLabels(doer, issue, nil, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
notification.NotifyIssueChangeLabels(doer, issue, nil, nil) | |
notification.NotifyIssueChangeLabels(doer, issue, []*models.lable {label}, nil) |
same here ...
services/issue/label.go
Outdated
@@ -106,6 +58,6 @@ func RemoveLabel(issue *models.Issue, doer *models.User, label *models.Label) er | |||
return err | |||
} | |||
|
|||
sendLabelUpdatedWebhook(issue, doer) | |||
notification.NotifyIssueChangeLabels(doer, issue, nil, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
notification.NotifyIssueChangeLabels(doer, issue, nil, nil) | |
notification.NotifyIssueChangeLabels(doer, issue, nil, []*models.lable {label}) |
same here ...
@lunny optional changes aboth ... beside that approved :D |
Codecov Report
@@ Coverage Diff @@
## master #8749 +/- ##
==========================================
- Coverage 41.39% 41.38% -0.02%
==========================================
Files 541 541
Lines 69522 69522
==========================================
- Hits 28780 28772 -8
- Misses 37048 37056 +8
Partials 3694 3694
Continue to review full report at Codecov.
|
@@ -398,3 +398,50 @@ func (m *webhookNotifier) NotifyDeleteComment(doer *models.User, comment *models | |||
go models.HookQueue.Add(comment.Issue.Repo.ID) | |||
} | |||
} | |||
|
|||
func (m *webhookNotifier) NotifyIssueChangeLabels(doer *models.User, issue *models.Issue, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that ClearLabels()
doesn't use this function. Is it a whole type of notification?
05949d4
to
63d4987
Compare
@6543 all done. |
63d4987
to
f923278
Compare
As title.