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

Remove release-note-none on PR w/deprecation label #15476

Conversation

@adshmh
Copy link
Contributor

adshmh commented Dec 4, 2019

The releasenote plugin now reacts to "kind/deprecation" label:

  1. If there is a "release-note-none" label, it is removed and
    a "do-not-merge/release-note-needed" label is added along with a comment
    explaining the reason.
  2. Setting the "release-note-none" label is not allowed on a PR which
    has the "kind/deprecation" label.

Fixes #15293

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 4, 2019

Hi @adshmh. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@matthyx

This comment has been minimized.

Copy link
Contributor

matthyx commented Dec 6, 2019

/ok-to-test

@matthyx

This comment has been minimized.

Copy link
Contributor

matthyx commented Dec 6, 2019

/lint

Copy link
Contributor

k8s-ci-robot left a comment

@matthyx: 0 warnings.

In response to this:

/lint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@matthyx

This comment has been minimized.

Copy link
Contributor

matthyx commented Dec 6, 2019

/lgtm
/assign @cblecker

@justaugustus

This comment has been minimized.

Copy link
Member

justaugustus commented Dec 15, 2019

/assign
/hold for my review on verbiage

Copy link
Member

cblecker left a comment

Thanks for doing this! A couple questions..

@@ -45,12 +45,15 @@ const (
releaseNoteFormat = `Adding the "%s" label because no release-note block was detected, please follow our [release note process](https://git.k8s.io/community/contributors/guide/release-notes.md) to remove it.`
parentReleaseNoteFormat = `All 'parent' PRs of a cherry-pick PR must have one of the %q or %q labels, or this PR must follow the standard/parent release note labeling requirement.`

actionRequiredNote = "action required"
actionRequiredNote = "action required"
deprecationNote = "kind/deprecation"

This comment has been minimized.

Copy link
@cblecker

cblecker Dec 22, 2019

Member

This is a label, not a note, correct?

This comment has been minimized.

Copy link
@adshmh

adshmh Dec 26, 2019

Author Contributor

Thanks for the review. Fixed.

@@ -237,6 +251,16 @@ func handlePR(gc githubClient, log *logrus.Entry, pr *github.PullRequestEvent) e
}
}

if labelToAdd == releaseNoteNone && prLabels.Has(deprecationNote) {
labelToAdd = ReleaseNoteLabelNeeded

This comment has been minimized.

Copy link
@cblecker

cblecker Dec 22, 2019

Member

This seems wrong.. why would we overwrite this, as opposed to adding some logic to determineReleaseNoteLabel?

This comment has been minimized.

Copy link
@adshmh

adshmh Dec 26, 2019

Author Contributor

Thank you for the review. Fixed.

@k8s-ci-robot k8s-ci-robot removed the lgtm label Dec 22, 2019
@adshmh adshmh force-pushed the adshmh:15293-Remove-Release-Note-None-on-deprecation-label branch from ca23c04 to f70f025 Dec 26, 2019
@k8s-ci-robot k8s-ci-robot added size/L and removed size/M labels Dec 26, 2019
@justaugustus

This comment has been minimized.

Copy link
Member

justaugustus commented Jan 8, 2020

Thanks for working on this, @adshmh!
/lgtm
/hold cancel

Pinging @cblecker for /approve.

@k8s-ci-robot k8s-ci-robot added lgtm and removed do-not-merge/hold labels Jan 8, 2020
Copy link
Member

cblecker left a comment

Couple of formatting nits, otherwise looks great! Thanks for this.

@@ -45,12 +45,15 @@ const (
releaseNoteFormat = `Adding the "%s" label because no release-note block was detected, please follow our [release note process](https://git.k8s.io/community/contributors/guide/release-notes.md) to remove it.`
parentReleaseNoteFormat = `All 'parent' PRs of a cherry-pick PR must have one of the %q or %q labels, or this PR must follow the standard/parent release note labeling requirement.`

actionRequiredNote = "action required"
actionRequiredNote = "action required"
deprecationLabel = "kind/deprecation"

This comment has been minimized.

Copy link
@cblecker

cblecker Jan 8, 2020

Member

Can you please group this label with the ones on L41-43?

This comment has been minimized.

Copy link
@adshmh

adshmh Jan 8, 2020

Author Contributor

Thank you for the review. Fixed.

actionRequiredNote = "action required"
actionRequiredNote = "action required"
deprecationLabel = "kind/deprecation"
releaseNoteDeprecationFormat = `Adding the "%s" label and removing any existing "%s" label because there is a "%s" label on the PR.`

This comment has been minimized.

Copy link
@cblecker

cblecker Jan 8, 2020

Member

Can you please group this Format with the ones on L45-46?

This comment has been minimized.

Copy link
@adshmh

adshmh Jan 8, 2020

Author Contributor

Thank you for the review. Fixed.

The releasenote plugin now reacts to "kind/deprecation" label:
   1. If there is a "release-note-none" label, it is removed and
a "do-not-merge/release-note-needed" label is added along with a comment
explaining the reason.
   2. Setting the "release-note-none" label is not allowed on a PR which
has the "kind/deprecation" label.

Signed-off-by: Arash Deshmeh <adeshmeh@ca.ibm.com>
@adshmh adshmh force-pushed the adshmh:15293-Remove-Release-Note-None-on-deprecation-label branch from f70f025 to 1634e89 Jan 8, 2020
@k8s-ci-robot k8s-ci-robot removed the lgtm label Jan 8, 2020
@justaugustus

This comment has been minimized.

Copy link
Member

justaugustus commented Jan 9, 2020

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Jan 9, 2020
@cblecker

This comment has been minimized.

Copy link
Member

cblecker commented Jan 9, 2020

/lgtm
/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jan 9, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adshmh, cblecker, justaugustus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 95b7e11 into kubernetes:master Jan 9, 2020
4 of 5 checks passed
4 of 5 checks passed
tide Not mergeable. Retesting: pull-test-infra-bazel
Details
cla/linuxfoundation adshmh authorized
Details
pull-test-infra-bazel Job succeeded.
Details
pull-test-infra-verify-file-perms Job succeeded.
Details
pull-test-infra-yamllint Job succeeded.
Details
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.