-
Notifications
You must be signed in to change notification settings - Fork 25
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
🌱 Allow tag prefixes in PR titles #44
🌱 Allow tag prefixes in PR titles #44
Conversation
}, | ||
{ | ||
name: "PR title with WIP", | ||
title: "WIP 📖 book: Use relative links in generate CRDs doc", |
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.
Would we allow WIP to me merged?
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 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:
- 📖 add ipam integration proposal cluster-api#6000
- https://prow.k8s.io/pr?query=is%3Apr+repo%3Akubernetes-sigs%2Fcluster-api+author%3Aschrej+head%3Aproposal%2Fipam
(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)
/assign @estroz |
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-.]*\]`) |
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.
var tagRegex = regexp.MustCompile(`^\[[\w-.]*\]`) | |
var tagRegex = regexp.MustCompile(`^\[[\w-\.]*\]`) |
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.
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.
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.
Doesn’t hurt I suppose. Thanks!
Signed-off-by: Stefan Büringer [email protected]
d547556
to
8d0f60f
Compare
/approve |
[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 |
@estroz Thank you! Could I get a release including this change / how does that work? :) |
I think just pushing semver and vX tags should do it. I’m AFK today so I can’t 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 :) |
@estroz friendly reminder :). If you've got some time, would be really great if you can create a release. |
Signed-off-by: Stefan Büringer [email protected]
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.