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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 17 additions & 2 deletions verify/type.go
Expand Up @@ -19,6 +19,7 @@ package main
import (
"fmt"
"regexp"
"strings"

"github.com/google/go-github/v32/github"

Expand All @@ -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-\.]*\]`)

type prTitleTypeError struct {
title string
}
Expand All @@ -53,8 +56,7 @@ More details can be found at [sigs.k8s.io/kubebuilder-release-tools/VERSIONING.m

// verifyPRType checks that the PR title contains a prefix that defines its type
func verifyPRType(pr *github.PullRequest) (string, string, error) {
// Remove the WIP prefix if found
title := wipRegex.ReplaceAllString(pr.GetTitle(), "")
title := trimTitle(pr.GetTitle())

prType, finalTitle := notes.PRTypeFromTitle(title)
if prType == notes.UncategorizedPR {
Expand All @@ -66,3 +68,16 @@ func verifyPRType(pr *github.PullRequest) (string, string, error) {
%s
`, finalTitle), nil
}

func trimTitle(title string) string {
// Remove the WIP prefix if found.
title = wipRegex.ReplaceAllString(title, "")

// Trim to remove spaces after WIP.
title = strings.TrimSpace(title)

// Remove a tag prefix if found.
title = tagRegex.ReplaceAllString(title, "")

return strings.TrimSpace(title)
}
60 changes: 60 additions & 0 deletions verify/type_test.go
@@ -0,0 +1,60 @@
/*
Copyright 2021 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package main

import "testing"

func Test_trimTitle(t *testing.T) {
tests := []struct {
name string
title string
want string
}{
{
name: "regular PR title",
title: "馃摉 book: Use relative links in generate CRDs doc",
want: "馃摉 book: Use relative links in generate CRDs doc",
},
{
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)

want: "馃摉 book: Use relative links in generate CRDs doc",
},
{
name: "PR title with [WIP]",
title: "[WIP] 馃摉 book: Use relative links in generate CRDs doc",
want: "馃摉 book: Use relative links in generate CRDs doc",
},
{
name: "PR title with [release-1.0]",
title: "[release-1.0] 馃摉 book: Use relative links in generate CRDs doc",
want: "馃摉 book: Use relative links in generate CRDs doc",
},
{
name: "PR title with [WIP][release-1.0]",
title: "[WIP][release-1.0] 馃摉 book: Use relative links in generate CRDs doc",
want: "馃摉 book: Use relative links in generate CRDs doc",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := trimTitle(tt.title); got != tt.want {
t.Errorf("trimTitle() = %v, want %v", got, tt.want)
}
})
}
}