You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/08/13 06:42:42 UTC
[GitHub] [pulsar-test-infra] tisonkun opened a new pull request, #56: avoid duplicate docbot comment
tisonkun opened a new pull request, #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56
We should send the comment only once when we add `doc/label-missing` label.
Otherwise, it may produce duplicate comments which act as spam. For example, https://github.com/apache/pulsar/pull/17009#issuecomment-1213636320.
cc @michaeljmarshall @maxsxu @nodece
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on a diff in pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#discussion_r945166378
##########
docbot/main.go:
##########
@@ -420,6 +425,11 @@ func (a *Action) onPullRequestLabeledOrUnlabeled() error {
// Add missing label
if a.config.GetEnableLabelMissing() && checkedCount == 0 {
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()]; exist {
+ logger.Infoln("Already added missing label.")
+ return fmt.Errorf("%s", MessageLabelMissing)
Review Comment:
```suggestion
return nil
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on a diff in pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#discussion_r945160245
##########
docbot/main.go:
##########
@@ -309,6 +309,11 @@ func (a *Action) onPullRequestOpenedOrEdited() error {
}
}
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()]; exist && checkedCount == 0 {
Review Comment:
Please move this section to line 326.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece merged pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
nodece merged PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] tisonkun commented on a diff in pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#discussion_r945165238
##########
docbot/main.go:
##########
@@ -309,6 +309,11 @@ func (a *Action) onPullRequestOpenedOrEdited() error {
}
}
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()]; exist && checkedCount == 0 {
Review Comment:
I can move this section inside `if a.config.GetEnableLabelMissing() && checkedCount == 0`, but cannot after `AddLabelsToIssue`. Otherwise, the docbot will never create comment because we check label missing just after we add it, and it will be always true.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] tisonkun commented on pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#issuecomment-1215319343
ping @maxsxu @michaeljmarshall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on a diff in pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#discussion_r945161884
##########
docbot/main.go:
##########
@@ -418,6 +423,11 @@ func (a *Action) onPullRequestLabeledOrUnlabeled() error {
}
}
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()]; exist && checkedCount == 0 {
Review Comment:
Please move this section to line 411.
##########
docbot/main.go:
##########
@@ -418,6 +423,11 @@ func (a *Action) onPullRequestLabeledOrUnlabeled() error {
}
}
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()]; exist && checkedCount == 0 {
Review Comment:
Please move this section to line 441.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] tisonkun commented on pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#issuecomment-1216306868
@maxsxu @nodece thanks for your review! Could you help on merging this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on a diff in pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#discussion_r945166295
##########
docbot/main.go:
##########
@@ -311,6 +311,11 @@ func (a *Action) onPullRequestOpenedOrEdited() error {
// Add missing label
if a.config.GetEnableLabelMissing() && checkedCount == 0 {
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()]; exist {
+ logger.Infoln("Already added missing label.")
+ return fmt.Errorf("%s", MessageLabelMissing)
Review Comment:
```suggestion
return nil;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] tisonkun commented on a diff in pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#discussion_r945165322
##########
docbot/main.go:
##########
@@ -309,6 +309,11 @@ func (a *Action) onPullRequestOpenedOrEdited() error {
}
}
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()]; exist && checkedCount == 0 {
Review Comment:
Updated at 1cab1c23a53446c107d594f177e945e5ffa09733
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#issuecomment-1214186102
Sorry that I forget to review the return value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] tisonkun commented on a diff in pull request #56: avoid duplicate docbot comment
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #56:
URL: https://github.com/apache/pulsar-test-infra/pull/56#discussion_r945234643
##########
docbot/main.go:
##########
@@ -311,6 +311,11 @@ func (a *Action) onPullRequestOpenedOrEdited() error {
// Add missing label
if a.config.GetEnableLabelMissing() && checkedCount == 0 {
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()]; exist {
+ logger.Infoln("Already added missing label.")
+ return fmt.Errorf("%s", MessageLabelMissing)
Review Comment:
We should return the error here. Still the pr needs a valid doc label. We just skip the create comment step.
##########
docbot/main.go:
##########
@@ -420,6 +425,11 @@ func (a *Action) onPullRequestLabeledOrUnlabeled() error {
// Add missing label
if a.config.GetEnableLabelMissing() && checkedCount == 0 {
+ if _, exist := currentLabelsSet[a.config.GetLabelMissing()]; exist {
+ logger.Infoln("Already added missing label.")
+ return fmt.Errorf("%s", MessageLabelMissing)
Review Comment:
See https://github.com/apache/pulsar-test-infra/pull/56#discussion_r945234643
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org