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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃尡 Allow tag prefixes in PR titles #44

Conversation

sbueringer
Copy link
Member

@sbueringer sbueringer commented Feb 1, 2022

Signed-off-by: Stefan B眉ringer buringerst@vmware.com

Fixes #43

Given that WIP and [WIP] prefixes are already dropped today, I'm not sure if we have to make this an optional behavior as originally discussed on the issue.

In case of CAPI we could implement handling of the now allowed prefixes (including for WIP) in the release note generation tool before bumping to a new kubebuilder-release-tools release.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 1, 2022
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 1, 2022
@sbueringer sbueringer changed the title Allow tag prefixes in PR titles 馃尡 Allow tag prefixes in PR titles Feb 1, 2022
@sbueringer
Copy link
Member Author

},
{
name: "PR title with WIP",
title: "WIP 馃摉 book: Use relative links in generate CRDs doc",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we allow WIP to me merged?

Copy link
Member Author

@sbueringer sbueringer Feb 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's just not the responsibility of this check to also produce errors for that case (previous version of that code allowed it too).

Independent of that GitHub check the Prow WIP label plugin adds a do-not-merge/work-in-progress label. Tide is then configured to block on this label.

Example:

(The good thing about the WIP label plugin is that it also adds the label on draft PRs and not only if the PR title contains WIP)

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 1, 2022
@sbueringer
Copy link
Member Author

/assign @estroz
Can you take a look, if you have some time? Thank you!

verify/type.go Outdated
@@ -28,6 +29,8 @@ import (
// Extracted from kubernetes/test-infra/prow/plugins/wip/wip-label.go
var wipRegex = regexp.MustCompile(`(?i)^\W?WIP\W`)

var tagRegex = regexp.MustCompile(`^\[[\w-.]*\]`)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var tagRegex = regexp.MustCompile(`^\[[\w-.]*\]`)
var tagRegex = regexp.MustCompile(`^\[[\w-\.]*\]`)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the review. Done.

Intellij tells me that the \ is redundant. It seems to work both ways. I've added it as I would also have expected that it's needed.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn鈥檛 hurt I suppose. Thanks!

Signed-off-by: Stefan B眉ringer buringerst@vmware.com
@sbueringer sbueringer force-pushed the pr-pr-verify-allow-tag-prefixes branch from d547556 to 8d0f60f Compare February 16, 2022 19:20
@estroz
Copy link

estroz commented Feb 16, 2022

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 16, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: estroz, sbueringer, vincepri

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 3892c77 into kubernetes-sigs:master Feb 16, 2022
@sbueringer
Copy link
Member Author

@estroz Thank you!

Could I get a release including this change / how does that work? :)

@sbueringer sbueringer deleted the pr-pr-verify-allow-tag-prefixes branch February 16, 2022 19:28
@estroz
Copy link

estroz commented Feb 16, 2022

I think just pushing semver and vX tags should do it. I鈥檓 AFK today so I can鈥檛 push until tonight. You can also just change download scripts to target the latest master commit.

@sbueringer
Copy link
Member Author

I think just pushing semver and vX tags should do it. I鈥檓 AFK today so I can鈥檛 push until tonight. You can also just change download scripts to target the latest master commit.

Just read the top-level README. I think you're right.

No rush, later would be absolutely great :)

@sbueringer
Copy link
Member Author

@estroz friendly reminder :). If you've got some time, would be really great if you can create a release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow "tag prefixes" in PR titles
5 participants