diff --git a/README.md b/README.md index cfebdf4..fd6712e 100644 --- a/README.md +++ b/README.md @@ -348,12 +348,15 @@ For example, given this `.github/labeler.yml`: ```yaml - label: "S" - size-below: 10 + size: + below: 10 - label: "M" - size-above: 9 - size-below: 100 + size: + above: 9 + below: 100 - label: "L" - size-above: 100 + size: + above: 100 ``` These would be the labels assigned to some PRs, based on their size as @@ -365,6 +368,11 @@ reported by the [GitHub API](https://developer.github.com/v3/pulls). |Second example|5|42|M| |Third example|68|148|L| +**NOTICE** the old format for specifying size properties (`size-above` +and `size-below`) has been deprecated. The action will continue +supporting old configs for now, but users are encouraged to migrate to +the new configuration schema. + ### Title This condition is satisfied when the title matches on the given regex. diff --git a/cmd/action.go b/cmd/action.go index 25e0a7c..1921d9e 100644 --- a/cmd/action.go +++ b/cmd/action.go @@ -34,7 +34,7 @@ func main() { return } - config, err := getLabelerConfig(configRaw) + config, err := getLabelerConfigV1(configRaw) if err != nil { return } @@ -89,15 +89,15 @@ func getRepoFile(gh *github.Client, repo, file, sha string) (*[]byte, error) { return &raw, err } -// getLabelerConfig builds a LabelerConfigV1 from a raw yaml -func getLabelerConfig(configRaw *[]byte) (*labeler.LabelerConfigV1, error) { +// getLabelerConfigV1 builds a LabelerConfigV1 from a raw yaml +func getLabelerConfigV1(configRaw *[]byte) (*labeler.LabelerConfigV1, error) { var c labeler.LabelerConfigV1 err := yaml.Unmarshal(*configRaw, &c) if err != nil { log.Printf("Unable to unmarshall config %s: ", err) } if c.Version == 0 { - c, err = getLabelerConfigV1(configRaw) + c, err = getLabelerConfigV0(configRaw) if err != nil { log.Printf("Unable to unmarshall legacy config %s: ", err) } @@ -105,7 +105,7 @@ func getLabelerConfig(configRaw *[]byte) (*labeler.LabelerConfigV1, error) { return &c, err } -func getLabelerConfigV1(configRaw *[]byte) (labeler.LabelerConfigV1, error) { +func getLabelerConfigV0(configRaw *[]byte) (labeler.LabelerConfigV1, error) { // Load v0 var oldCfg map[string]labeler.LabelMatcher diff --git a/cmd/action_test.go b/cmd/action_test.go index a742914..ae188f7 100644 --- a/cmd/action_test.go +++ b/cmd/action_test.go @@ -3,10 +3,12 @@ package main import ( "io/ioutil" "os" + "reflect" "testing" "github.com/google/go-cmp/cmp" l "github.com/srvaroa/labeler/pkg" + labeler "github.com/srvaroa/labeler/pkg" ) func TestGetLabelerConfigV0(t *testing.T) { @@ -22,7 +24,7 @@ func TestGetLabelerConfigV0(t *testing.T) { } var c *l.LabelerConfigV1 - c, err = getLabelerConfig(&contents) + c, err = getLabelerConfigV1(&contents) if err != nil { t.Fatal(err) } @@ -85,7 +87,7 @@ func TestGetLabelerConfigV1(t *testing.T) { } var c *l.LabelerConfigV1 - c, err = getLabelerConfig(&contents) + c, err = getLabelerConfigV1(&contents) if err != nil { t.Fatal(err) } @@ -165,7 +167,7 @@ func TestGetLabelerConfigV1WithIssues(t *testing.T) { } var c *l.LabelerConfigV1 - c, err = getLabelerConfig(&contents) + c, err = getLabelerConfigV1(&contents) if err != nil { t.Fatal(err) } @@ -189,6 +191,47 @@ func TestGetLabelerConfigV1WithIssues(t *testing.T) { } } +func TestGetLabelerConfigV1WithCompositeSize(t *testing.T) { + + file, err := os.Open("../test_data/config_v1_composite_size.yml") + if err != nil { + t.Fatal(err) + } + + contents, err := ioutil.ReadAll(file) + if err != nil { + t.Fatal(err) + } + + var c *l.LabelerConfigV1 + c, err = getLabelerConfigV1(&contents) + if err != nil { + t.Fatal(err) + } + + expect := l.LabelerConfigV1{ + Version: 1, + Labels: []l.LabelMatcher{ + { + Label: "S", + SizeAbove: "1", + SizeBelow: "10", + }, + { + Label: "M", + Size: &labeler.SizeConfig{ + Above: "9", + Below: "100", + }, + }, + }, + } + + if !reflect.DeepEqual(expect, *c) { + t.Fatalf("\nExpect: %#v \nGot: %#v", expect, *c) + } +} + func TestGetLabelerConfig2V1(t *testing.T) { file, err := os.Open("../test_data/config2_v1.yml") @@ -202,7 +245,7 @@ func TestGetLabelerConfig2V1(t *testing.T) { } var c *l.LabelerConfigV1 - c, err = getLabelerConfig(&contents) + c, err = getLabelerConfigV1(&contents) if err != nil { t.Fatal(err) } diff --git a/pkg/condition_size.go b/pkg/condition_size.go index 037e316..1e4d24e 100644 --- a/pkg/condition_size.go +++ b/pkg/condition_size.go @@ -16,15 +16,34 @@ func SizeCondition() Condition { return target.ghPR != nil }, Evaluate: func(target *Target, matcher LabelMatcher) (bool, error) { - if len(matcher.SizeBelow) == 0 && len(matcher.SizeAbove) == 0 { - return false, fmt.Errorf("size-above and size-below are not set in config") + + if isNewConfig(matcher) && isOldConfig(matcher) { + log.Printf("WARNING: you are using both the old " + + "`size-above`/`size-below` settings together with " + + "the newer `size`. You should use only the latter. " + + "This condition will apply the configurations set in `Size` " + + "and ignore the rest") + } + + realMatcher := matcher.Size + if realMatcher == nil { + if matcher.SizeBelow == "" && matcher.SizeAbove == "" { + return false, fmt.Errorf("no size conditions are set in config") + } + realMatcher = &SizeConfig{ + Above: matcher.SizeAbove, + Below: matcher.SizeBelow, + } } - upperBound, err := strconv.ParseInt(matcher.SizeBelow, 0, 64) + + log.Printf("Checking PR size using config: %+v", realMatcher) + + upperBound, err := strconv.ParseInt(realMatcher.Below, 0, 64) if err != nil { upperBound = math.MaxInt64 log.Printf("Upper boundary set to %d (config has invalid or empty value)", upperBound) } - lowerBound, err := strconv.ParseInt(matcher.SizeAbove, 0, 32) + lowerBound, err := strconv.ParseInt(realMatcher.Above, 0, 32) if err != nil || lowerBound < 0 { lowerBound = 0 log.Printf("Lower boundary set to 0 (config has invalid or empty value)") @@ -36,3 +55,11 @@ func SizeCondition() Condition { }, } } + +func isNewConfig(matcher LabelMatcher) bool { + return matcher.Size != nil +} + +func isOldConfig(matcher LabelMatcher) bool { + return matcher.SizeAbove != "" || matcher.SizeBelow != "" +} diff --git a/pkg/labeler.go b/pkg/labeler.go index caddc19..55192d2 100644 --- a/pkg/labeler.go +++ b/pkg/labeler.go @@ -8,6 +8,11 @@ import ( gh "github.com/google/go-github/v50/github" ) +type SizeConfig struct { + Above string + Below string +} + type LabelMatcher struct { AuthorCanMerge string `yaml:"author-can-merge"` Authors []string @@ -19,9 +24,16 @@ type LabelMatcher struct { Label string Mergeable string Negate bool - SizeAbove string `yaml:"size-above"` - SizeBelow string `yaml:"size-below"` - Title string + Size *SizeConfig + // size-legacy + // These two are unused in the codebase (they get copied inside + // the Size object), but we keep them to respect backwards + // compatiblity parsing older configs without adding more + // complexity. + SizeAbove string `yaml:"size-above"` + SizeBelow string `yaml:"size-below"` + // size-legacy + Title string } type LabelerConfigV0 map[string]LabelMatcher diff --git a/pkg/labeler_test.go b/pkg/labeler_test.go index 9ded733..e1b6fb8 100644 --- a/pkg/labeler_test.go +++ b/pkg/labeler_test.go @@ -258,6 +258,25 @@ func TestHandleEvent(t *testing.T) { initialLabels: []string{}, expectedLabels: []string{"M"}, }, + { + event: "pull_request", + payloads: []string{"mid_pr"}, + name: "Test the size_below and size_above rules with new composite config", + config: LabelerConfigV1{ + Version: 1, + Labels: []LabelMatcher{ + { + Label: "M", + Size: &SizeConfig{ + Above: "9", + Below: "100", + }, + }, + }, + }, + initialLabels: []string{}, + expectedLabels: []string{"M"}, + }, { event: "pull_request", payloads: []string{"big_pr"}, diff --git a/test_data/config_v1_composite_size.yml b/test_data/config_v1_composite_size.yml new file mode 100644 index 0000000..57acf4f --- /dev/null +++ b/test_data/config_v1_composite_size.yml @@ -0,0 +1,9 @@ +version: 1 +labels: + - label: S + size-below: 10 + size-above: 1 + - label: M + size: + above: 9 + below: 100