diff --git a/README.md b/README.md index 9689c801e..a2cae9da4 100644 --- a/README.md +++ b/README.md @@ -8,9 +8,9 @@ status checks are successful and required reviews are provided. Additionally, `bulldozer` can: -- Only merge pull requests that match a whitelist condition, like having a +- Only merge pull requests that match certain conditions, like having a specific label or comment -- Ignore pull requests that match a blacklist condition, like having a specific +- Ignore pull requests that match certain conditions, like having a specific label or comment - Automatically keep pull request branches up-to-date by merging in the target branch @@ -47,8 +47,8 @@ including required status checks and required reviews, are respected. It also means that you _must_ enable branch protection to prevent `bulldozer` from immediately merging every pull request. -Only pull requests matching the whitelist conditions (or _not_ matching the -blacklist conditions) are considered for merging. `bulldozer` is event-driven, +Only pull requests matching the trigger conditions (or _not_ matching +ignore conditions) are considered for merging. `bulldozer` is event-driven, which means it will usually merge a pull request within a few seconds of the pull request satisfying all preconditions. @@ -74,33 +74,33 @@ version: 1 # "merge" defines how and when pull requests are merged. If the section is # missing, bulldozer will consider all pull requests and use default settings. merge: - # "whitelist" defines the set of pull requests considered by bulldozer. If + # "trigger" defines the set of pull requests considered by bulldozer. If # the section is missing, bulldozer considers all pull requests not excluded - # by the blacklist. - whitelist: + # by the ignore conditions. + trigger: # Pull requests with any of these labels (case-insensitive) are added to - # the whitelist. + # the trigger. labels: ["merge when ready"] # Pull requests where the body or any comment contains any of these - # substrings are added to the whitelist. + # substrings are added to the trigger. comment_substrings: ["==MERGE_WHEN_READY=="] # Pull requests where any comment matches one of these exact strings are - # added to the whitelist. + # added to the trigger. comments: ["Please merge this pull request!"] # Pull requests where the body contains any of these substrings are added - # to the whitelist. + # to the trigger. pr_body_substrings: ["==MERGE_WHEN_READY=="] - # Pull requests targeting any of these branches are added to the whitelist. + # Pull requests targeting any of these branches are added to the trigger. branches: ["develop"] - # "blacklist" defines the set of pull request ignored by bulldozer. If the + # "ignore" defines the set of pull request ignored by bulldozer. If the # section is missing, bulldozer considers all pull requests. It takes the - # same keys as the "whitelist" section. - blacklist: + # same keys as the "trigger" section. + ignore: labels: ["do not merge"] comment_substrings: ["==DO_NOT_MERGE=="] @@ -148,23 +148,23 @@ merge: # "update" defines how and when to update pull request branches. Unlike with # merges, if this section is missing, bulldozer will not update any pull requests. update: - # "whitelist" defines the set of pull requests that should be updated by - # bulldozer. It accepts the same keys as the whitelist in the "merge" block. - whitelist: + # "trigger" defines the set of pull requests that should be updated by + # bulldozer. It accepts the same keys as the trigger in the "merge" block. + trigger: labels: ["WIP", "Update Me"] - # "blacklist" defines the set of pull requests that should not be updated by - # bulldozer. It accepts the same keys as the blacklist in the "merge" block. - blacklist: + # "ignore" defines the set of pull requests that should not be updated by + # bulldozer. It accepts the same keys as the ignore in the "merge" block. + ignore: labels: ["Do Not Update"] ``` ## FAQ -#### Can I specify both a `blacklist` and `whitelist`? +#### Can I specify both `ignore` and `trigger`? -Yes. If both `blacklist` and `whitelist` are specified, bulldozer will attempt to match -on both. In cases where both match, `blacklist` will take precedence. +Yes. If both `ignore` and `trigger` are specified, bulldozer will attempt to match +on both. In cases where both match, `ignore` will take precedence. #### Can I specify the body of the commit when using the `squash` strategy? @@ -203,13 +203,13 @@ It will be used only when your repo config file does not exist. ```yaml options: default_repository_config: - blacklist: + ignore: labels: ["do not merge"] # or any other available config. ``` #### Bulldozer isn't merging my commit when it should, what could be happening? -Bulldozer will attempt to merge a branch whenever it passes the whitelist/blacklist +Bulldozer will attempt to merge a branch whenever it passes the trigger/ignore criteria. GitHub may prevent it from merging a branch in certain conditions, some of which are to be expected, and others that may be caused by mis-configuring Bulldozer. @@ -219,7 +219,7 @@ which are to be expected, and others that may be caused by mis-configuring Bulld repository settings * Branch protection rules are preventing `bulldozer[bot]` from [pushing to the branch][push restrictions]. Github apps can be added to the list of restricted - push users, so you can whitelist bulldozer specifically for your repo. + push users, so you can allow Bulldozer specifically for your repo. [push restrictions]: https://help.github.com/articles/about-branch-restrictions/ [a workaround]: #can-bulldozer-work-with-push-restrictions-on-branches diff --git a/bulldozer/config_fetcher.go b/bulldozer/config_fetcher.go index 57b3067ac..8904b137e 100644 --- a/bulldozer/config_fetcher.go +++ b/bulldozer/config_fetcher.go @@ -148,6 +148,20 @@ func (cf *ConfigFetcher) unmarshalConfig(bytes []byte) (*Config, error) { return nil, errors.Wrapf(err, "failed to unmarshal configuration") } + // Merge old signals configurations if they exist when the new values aren't present + if config.Merge.Blacklist.Enabled() && !config.Merge.Ignore.Enabled() { + config.Merge.Ignore = config.Merge.Blacklist + } + if config.Merge.Whitelist.Enabled() && !config.Merge.Trigger.Enabled() { + config.Merge.Trigger = config.Merge.Whitelist + } + if config.Update.Blacklist.Enabled() && !config.Update.Ignore.Enabled() { + config.Update.Ignore = config.Update.Blacklist + } + if config.Update.Whitelist.Enabled() && !config.Update.Trigger.Enabled() { + config.Update.Trigger = config.Update.Whitelist + } + if config.Version != 1 { return nil, errors.Errorf("unexpected version '%d', expected 1", config.Version) } @@ -167,12 +181,12 @@ func (cf *ConfigFetcher) unmarshalConfigV0(bytes []byte) (*Config, error) { config = Config{ Version: 1, Update: UpdateConfig{ - Whitelist: Signals{ + Trigger: Signals{ Labels: []string{"update me", "update-me", "update_me"}, }, }, Merge: MergeConfig{ - Whitelist: Signals{ + Trigger: Signals{ Labels: []string{"merge when ready", "merge-when-ready", "merge_when_ready"}, }, DeleteAfterMerge: configv0.DeleteAfterMerge, @@ -188,12 +202,12 @@ func (cf *ConfigFetcher) unmarshalConfigV0(bytes []byte) (*Config, error) { config = Config{ Version: 1, Update: UpdateConfig{ - Whitelist: Signals{ + Trigger: Signals{ Labels: []string{"update me", "update-me", "update_me"}, }, }, Merge: MergeConfig{ - Blacklist: Signals{ + Ignore: Signals{ Labels: []string{"wip", "do not merge", "do-not-merge", "do_not_merge"}, }, DeleteAfterMerge: configv0.DeleteAfterMerge, @@ -209,12 +223,12 @@ func (cf *ConfigFetcher) unmarshalConfigV0(bytes []byte) (*Config, error) { config = Config{ Version: 1, Update: UpdateConfig{ - Whitelist: Signals{ + Trigger: Signals{ Labels: []string{"update me", "update-me", "update_me"}, }, }, Merge: MergeConfig{ - Whitelist: Signals{ + Trigger: Signals{ CommentSubstrings: []string{"==MERGE_WHEN_READY=="}, }, DeleteAfterMerge: configv0.DeleteAfterMerge, diff --git a/bulldozer/config_fetcher_test.go b/bulldozer/config_fetcher_test.go new file mode 100644 index 000000000..dbe901a61 --- /dev/null +++ b/bulldozer/config_fetcher_test.go @@ -0,0 +1,160 @@ +// Copyright 2020 Palantir Technologies, Inc. +// +// 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 bulldozer + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMarshalling(t *testing.T) { + t.Run("parseNewConfig", func(t *testing.T) { + cf := NewConfigFetcher("", []string{""}, nil) + + config := ` +version: 1 + +merge: + trigger: + labels: ["merge when ready"] + comment_substrings: ["==MERGE_WHEN_READY=="] + ignore: + labels: ["do not merge"] + comment_substrings: ["==DO_NOT_MERGE=="] + method: squash + options: + squash: + body: summarize_commits + delete_after_merge: true + +update: + trigger: + labels: ["wip", "update me"] + ignore: + labels: ["do not update"] +` + + actual, err := cf.unmarshalConfig([]byte(config)) + require.Nil(t, err) + assert.Equal(t, Signals{ + Labels: []string{"merge when ready"}, + CommentSubstrings: []string{"==MERGE_WHEN_READY=="}, + }, actual.Merge.Trigger) + assert.Equal(t, Signals{ + Labels: []string{"do not merge"}, + CommentSubstrings: []string{"==DO_NOT_MERGE=="}, + }, actual.Merge.Ignore) + }) + + t.Run("parseExisting", func(t *testing.T) { + cf := NewConfigFetcher("", []string{""}, nil) + + config := ` +version: 1 + +merge: + whitelist: + labels: ["merge when ready"] + comment_substrings: ["==OLD_MERGE_WHEN_READY=="] + blacklist: + labels: ["do not merge"] + comment_substrings: ["==OLD_DO_NOT_MERGE=="] + method: squash + options: + squash: + body: summarize_commits + delete_after_merge: true + +update: + whitelist: + labels: ["wip", "update me"] + blacklist: + labels: ["do not update"] +` + + actual, err := cf.unmarshalConfig([]byte(config)) + require.Nil(t, err) + + assert.Equal(t, Signals{ + Labels: []string{"merge when ready"}, + CommentSubstrings: []string{"==OLD_MERGE_WHEN_READY=="}, + }, actual.Merge.Trigger) + assert.Equal(t, Signals{ + Labels: []string{"do not merge"}, + CommentSubstrings: []string{"==OLD_DO_NOT_MERGE=="}, + }, actual.Merge.Ignore) + + assert.Equal(t, Signals{ + Labels: []string{"wip", "update me"}, + }, actual.Update.Trigger) + assert.Equal(t, Signals{ + Labels: []string{"do not update"}, + }, actual.Update.Ignore) + }) + + t.Run("ignoresOldConfig", func(t *testing.T) { + cf := NewConfigFetcher("", []string{""}, nil) + + config := ` +version: 1 + +merge: + trigger: + labels: ["mwr"] + ignore: + labels: ["new dnm"] + whitelist: + labels: ["merge when ready"] + comment_substrings: ["==OLD_MERGE_WHEN_READY=="] + blacklist: + labels: ["do not merge"] + comment_substrings: ["==OLD_DO_NOT_MERGE=="] + method: squash + options: + squash: + body: summarize_commits + delete_after_merge: true + +update: + trigger: + labels: ["new wip"] + ignore: + labels: ["new dnu"] + whitelist: + labels: ["wip", "update me"] + blacklist: + labels: ["do not update"] +` + + actual, err := cf.unmarshalConfig([]byte(config)) + require.Nil(t, err) + + assert.Equal(t, Signals{ + Labels: []string{"mwr"}, + }, actual.Merge.Trigger) + assert.Equal(t, Signals{ + Labels: []string{"new dnm"}, + }, actual.Merge.Ignore) + + assert.Equal(t, Signals{ + Labels: []string{"new wip"}, + }, actual.Update.Trigger) + assert.Equal(t, Signals{ + Labels: []string{"new dnu"}, + }, actual.Update.Ignore) + }) +} diff --git a/bulldozer/config_v1.go b/bulldozer/config_v1.go index 3d4832c3c..3d1875d73 100644 --- a/bulldozer/config_v1.go +++ b/bulldozer/config_v1.go @@ -34,8 +34,12 @@ const ( ) type MergeConfig struct { - Whitelist Signals `yaml:"whitelist"` + Trigger Signals `yaml:"trigger"` + Ignore Signals `yaml:"ignore"` + + // Blacklist and Whitelist are legacy options that will be disabled in a future v2 format Blacklist Signals `yaml:"blacklist"` + Whitelist Signals `yaml:"whitelist"` DeleteAfterMerge bool `yaml:"delete_after_merge"` @@ -60,8 +64,12 @@ type SquashOptions struct { } type UpdateConfig struct { - Whitelist Signals `yaml:"whitelist"` + Trigger Signals `yaml:"trigger"` + Ignore Signals `yaml:"ignore"` + + // Blacklist and Whitelist are legacy options that will be disabled in a future v2 format Blacklist Signals `yaml:"blacklist"` + Whitelist Signals `yaml:"whitelist"` } type Config struct { diff --git a/bulldozer/evaluate.go b/bulldozer/evaluate.go index 9b1e4e8c8..1b167eafc 100644 --- a/bulldozer/evaluate.go +++ b/bulldozer/evaluate.go @@ -24,23 +24,23 @@ import ( "github.com/palantir/bulldozer/pull" ) -// IsPRBlacklisted returns true if the PR is identified as blacklisted, +// IsPRIgnored returns true if the PR is identified as ignored, // false otherwise. Additionally, a description of the reason will be returned. -func IsPRBlacklisted(ctx context.Context, pullCtx pull.Context, config Signals) (bool, string, error) { - matches, reason, err := config.Matches(ctx, pullCtx, "blacklist") +func IsPRIgnored(ctx context.Context, pullCtx pull.Context, config Signals) (bool, string, error) { + matches, reason, err := config.Matches(ctx, pullCtx, "ignored") if err != nil { - // blacklist must always fail closed (matches on error) + // ignore must always fail closed (matches on error) matches = true } return matches, reason, err } -// IsPRWhitelisted returns true if the PR is identified as whitelisted, +// IsPRTriggered returns true if the PR is identified as triggered, // false otherwise. Additionally, a description of the reason will be returned. -func IsPRWhitelisted(ctx context.Context, pullCtx pull.Context, config Signals) (bool, string, error) { - matches, reason, err := config.Matches(ctx, pullCtx, "whitelist") +func IsPRTriggered(ctx context.Context, pullCtx pull.Context, config Signals) (bool, string, error) { + matches, reason, err := config.Matches(ctx, pullCtx, "triggered") if err != nil { - // whitelist must always fail closed (no match on error) + // trigger must always fail closed (no match on error) return false, reason, err } return matches, reason, err @@ -89,32 +89,32 @@ func statusSetDifference(required, actual []string) []string { func ShouldMergePR(ctx context.Context, pullCtx pull.Context, mergeConfig MergeConfig) (bool, error) { logger := zerolog.Ctx(ctx) - if mergeConfig.Blacklist.Enabled() { - blacklisted, reason, err := IsPRBlacklisted(ctx, pullCtx, mergeConfig.Blacklist) + if mergeConfig.Ignore.Enabled() { + ignored, reason, err := IsPRIgnored(ctx, pullCtx, mergeConfig.Ignore) if err != nil { - return false, errors.Wrap(err, "failed to determine if pull request is blacklisted") + return false, errors.Wrap(err, "failed to determine if pull request is ignored") } - if blacklisted { - logger.Debug().Msgf("%s is deemed not mergeable because blacklisting is enabled and %s", pullCtx.Locator(), reason) + if ignored { + logger.Debug().Msgf("%s is deemed not mergeable because ignoring is enabled and %s", pullCtx.Locator(), reason) return false, nil } } else { - logger.Debug().Msg("blacklisting is not enabled") + logger.Debug().Msg("ignoring is not enabled") } - if mergeConfig.Whitelist.Enabled() { - whitelisted, reason, err := IsPRWhitelisted(ctx, pullCtx, mergeConfig.Whitelist) + if mergeConfig.Trigger.Enabled() { + triggered, reason, err := IsPRTriggered(ctx, pullCtx, mergeConfig.Trigger) if err != nil { - return false, errors.Wrap(err, "failed to determine if pull request is whitelisted") + return false, errors.Wrap(err, "failed to determine if pull request is triggered") } - if !whitelisted { - logger.Debug().Msgf("%s is deemed not mergeable because whitelisting is enabled and no whitelist signal detected", pullCtx.Locator()) + if !triggered { + logger.Debug().Msgf("%s is deemed not mergeable because triggering is enabled and no trigger signal detected", pullCtx.Locator()) return false, nil } - logger.Debug().Msgf("%s is whitelisted because whitelisting is enabled and %s", pullCtx.Locator(), reason) + logger.Debug().Msgf("%s is triggered because triggering is enabled and %s", pullCtx.Locator(), reason) } else { - logger.Debug().Msg("whitelisting is not enabled") + logger.Debug().Msg("triggering is not enabled") } requiredStatuses, err := pullCtx.RequiredStatuses(ctx) @@ -135,6 +135,5 @@ func ShouldMergePR(ctx context.Context, pullCtx pull.Context, mergeConfig MergeC } // Ignore required reviews and try a merge (which may fail with a 4XX). - return true, nil } diff --git a/bulldozer/evaluate_test.go b/bulldozer/evaluate_test.go index 6c147f64e..09e362688 100644 --- a/bulldozer/evaluate_test.go +++ b/bulldozer/evaluate_test.go @@ -27,14 +27,14 @@ import ( func TestSimpleXListed(t *testing.T) { mergeConfig := MergeConfig{ - Whitelist: Signals{ + Trigger: Signals{ Labels: []string{"LABEL_MERGE"}, Comments: []string{"FULL_COMMENT_PLZ_MERGE"}, CommentSubstrings: []string{":+1:"}, PRBodySubstrings: []string{"BODY_MERGE_PLZ"}, Branches: []string{"develop"}, }, - Blacklist: Signals{ + Ignore: Signals{ Labels: []string{"LABEL_NOMERGE"}, Comments: []string{"NO_WAY"}, CommentSubstrings: []string{":-1:"}, @@ -45,55 +45,55 @@ func TestSimpleXListed(t *testing.T) { ctx := context.Background() - t.Run("errCommentFailsClosedBlacklist", func(t *testing.T) { + t.Run("errCommentFailsClosedDenylist", func(t *testing.T) { pc := &pulltest.MockPullContext{ CommentErrValue: errors.New("failure"), } - actualBlacklist, _, err := IsPRBlacklisted(ctx, pc, mergeConfig.Blacklist) + actual, _, err := IsPRIgnored(ctx, pc, mergeConfig.Ignore) require.NotNil(t, err) - assert.True(t, actualBlacklist) + assert.True(t, actual) }) - t.Run("errCommentFailsClosedWhitelist", func(t *testing.T) { + t.Run("errCommentFailsClosedAllowlist", func(t *testing.T) { pc := &pulltest.MockPullContext{ CommentErrValue: errors.New("failure"), } - actualWhitelist, _, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist) + actual, _, err := IsPRTriggered(ctx, pc, mergeConfig.Trigger) require.NotNil(t, err) - assert.False(t, actualWhitelist) + assert.False(t, actual) }) - t.Run("errLabelFailsClosedWhitelist", func(t *testing.T) { + t.Run("errLabelFailsClosedAllowlist", func(t *testing.T) { pc := &pulltest.MockPullContext{ LabelErrValue: errors.New("failure"), } - actualWhitelist, _, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist) + actual, _, err := IsPRTriggered(ctx, pc, mergeConfig.Trigger) require.NotNil(t, err) - assert.False(t, actualWhitelist) + assert.False(t, actual) }) - t.Run("errCommentsFailsClosedWhitelist", func(t *testing.T) { + t.Run("errCommentsFailsClosedAllowlist", func(t *testing.T) { pc := &pulltest.MockPullContext{ CommentErrValue: errors.New("failure"), } - actualWhitelist, _, err := IsPRWhitelisted(ctx, pc, mergeConfig.Whitelist) + actual, _, err := IsPRTriggered(ctx, pc, mergeConfig.Trigger) require.NotNil(t, err) - assert.False(t, actualWhitelist) + assert.False(t, actual) }) } func TestShouldMerge(t *testing.T) { mergeConfig := MergeConfig{ - Whitelist: Signals{ + Trigger: Signals{ Labels: []string{"LABEL_MERGE", "LABEL2_MERGE"}, Comments: []string{"FULL_COMMENT_PLZ_MERGE"}, CommentSubstrings: []string{":+1:", ":y:"}, }, - Blacklist: Signals{ + Ignore: Signals{ Labels: []string{"LABEL_NOMERGE"}, Comments: []string{"NO_WAY"}, CommentSubstrings: []string{":-1:"}, @@ -167,7 +167,7 @@ func TestShouldMerge(t *testing.T) { assert.False(t, actualShouldMerge) }) - t.Run("blacklistOverridesWhitelist", func(t *testing.T) { + t.Run("ignoreOverridesAllowlist", func(t *testing.T) { pc := &pulltest.MockPullContext{ LabelValue: []string{"LABEL2_MERGE"}, CommentValue: []string{"NO_WAY"}, @@ -179,7 +179,7 @@ func TestShouldMerge(t *testing.T) { assert.False(t, actualShouldMerge) }) - t.Run("labelCausesBlacklist", func(t *testing.T) { + t.Run("labelCausesDenylist", func(t *testing.T) { pc := &pulltest.MockPullContext{ LabelValue: []string{"LABEL_NOMERGE"}, } @@ -190,7 +190,7 @@ func TestShouldMerge(t *testing.T) { assert.False(t, actualShouldMerge) }) - t.Run("labelCausesBlacklistCaseInsensitive", func(t *testing.T) { + t.Run("labelCausesDenylistCaseInsensitive", func(t *testing.T) { pc := &pulltest.MockPullContext{ LabelValue: []string{"LABEL_nomERGE"}, } @@ -201,7 +201,7 @@ func TestShouldMerge(t *testing.T) { assert.False(t, actualShouldMerge) }) - t.Run("substringCausesWhitelist", func(t *testing.T) { + t.Run("substringCausesAllowlist", func(t *testing.T) { pc := &pulltest.MockPullContext{ LabelValue: []string{"NOT_A_LABEL"}, CommentValue: []string{"a comment", "another comment", "this is good :+1: yep"}, @@ -213,7 +213,7 @@ func TestShouldMerge(t *testing.T) { assert.True(t, actualShouldMerge) }) - t.Run("substringCausesBlacklist", func(t *testing.T) { + t.Run("substringCausesDenylist", func(t *testing.T) { pc := &pulltest.MockPullContext{ LabelValue: []string{"LABEL_NOMERGE"}, CommentValue: []string{"a comment", "another comment", "this is no good nope\n\r:-1:"}, diff --git a/bulldozer/signals.go b/bulldozer/signals.go index 11657d979..c86337332 100644 --- a/bulldozer/signals.go +++ b/bulldozer/signals.go @@ -44,7 +44,7 @@ func (s *Signals) Enabled() bool { // Matches returns true if the pull request meets one or more signals. It also // returns a description of the signal that was met. The tag argument appears -// in this description and indicates the behavior (whitelist, blacklist) this +// in this description and indicates the behavior (trigger, ignore) this // set of signals is associated with. func (s *Signals) Matches(ctx context.Context, pullCtx pull.Context, tag string) (bool, string, error) { logger := zerolog.Ctx(ctx) diff --git a/bulldozer/update.go b/bulldozer/update.go index 12ae7a0cb..ce9696fe9 100644 --- a/bulldozer/update.go +++ b/bulldozer/update.go @@ -27,32 +27,32 @@ import ( func ShouldUpdatePR(ctx context.Context, pullCtx pull.Context, updateConfig UpdateConfig) (bool, error) { logger := zerolog.Ctx(ctx) - if !updateConfig.Blacklist.Enabled() && !updateConfig.Whitelist.Enabled() { + if !updateConfig.Ignore.Enabled() && !updateConfig.Trigger.Enabled() { return false, nil } - if updateConfig.Blacklist.Enabled() { - blacklisted, reason, err := IsPRBlacklisted(ctx, pullCtx, updateConfig.Blacklist) + if updateConfig.Ignore.Enabled() { + ignored, reason, err := IsPRIgnored(ctx, pullCtx, updateConfig.Ignore) if err != nil { - return false, errors.Wrap(err, "failed to determine if pull request is blacklisted") + return false, errors.Wrap(err, "failed to determine if pull request is ignored") } - if blacklisted { - logger.Debug().Msgf("%s is deemed not updateable because blacklisting is enabled and %s", pullCtx.Locator(), reason) + if ignored { + logger.Debug().Msgf("%s is deemed not updateable because ignoring is enabled and %s", pullCtx.Locator(), reason) return false, nil } } - if updateConfig.Whitelist.Enabled() { - whitelisted, reason, err := IsPRWhitelisted(ctx, pullCtx, updateConfig.Whitelist) + if updateConfig.Trigger.Enabled() { + triggered, reason, err := IsPRTriggered(ctx, pullCtx, updateConfig.Trigger) if err != nil { - return false, errors.Wrap(err, "failed to determine if pull request is whitelisted") + return false, errors.Wrap(err, "failed to determine if pull request is triggered") } - if !whitelisted { - logger.Debug().Msgf("%s is deemed not updateable because whitelisting is enabled and no whitelist signal detected", pullCtx.Locator()) + if !triggered { + logger.Debug().Msgf("%s is deemed not updateable because triggering is enabled and no trigger signal detected", pullCtx.Locator()) return false, nil } - logger.Debug().Msgf("%s is whitelisted because whitelisting is enabled and %s", pullCtx.Locator(), reason) + logger.Debug().Msgf("%s is triggered because triggering is enabled and %s", pullCtx.Locator(), reason) } return true, nil diff --git a/bulldozer/update_test.go b/bulldozer/update_test.go index 39575fadf..6052511e2 100644 --- a/bulldozer/update_test.go +++ b/bulldozer/update_test.go @@ -28,11 +28,11 @@ import ( func TestShouldUpdatePR(t *testing.T) { ctx := context.Background() testMatrix := []struct { - blacklistEnabled bool - blacklisted bool - whitelistEnabled bool - whitelisted bool - expectingUpdate bool + ignoreEnabled bool + ignored bool + triggerEnabled bool + triggered bool + expectingUpdate bool }{ {false, false, false, false, false}, {false, false, false, true, false}, @@ -53,32 +53,32 @@ func TestShouldUpdatePR(t *testing.T) { } for ndx, testCase := range testMatrix { - pullCtx, updateConfig := generateUpdateTestCase(testCase.blacklistEnabled, testCase.blacklisted, testCase.whitelistEnabled, testCase.whitelisted) + pullCtx, updateConfig := generateUpdateTestCase(testCase.ignoreEnabled, testCase.ignored, testCase.triggerEnabled, testCase.triggered) updating, err := ShouldUpdatePR(ctx, pullCtx, updateConfig) require.NoError(t, err) - msg := fmt.Sprintf("case %d - blacklistEnabled=%t blacklisted=%t whitelistEnabled=%t whitelisted=%t -> doUpdate=%t", - ndx, testCase.blacklistEnabled, testCase.blacklisted, testCase.whitelistEnabled, testCase.whitelisted, testCase.expectingUpdate) + msg := fmt.Sprintf("case %d - ignoreEnabled=%t ignored=%t triggerEnabled=%t triggered=%t -> doUpdate=%t", + ndx, testCase.ignoreEnabled, testCase.ignored, testCase.triggerEnabled, testCase.triggered, testCase.expectingUpdate) require.Equal(t, testCase.expectingUpdate, updating, msg) } } -func generateUpdateTestCase(blacklistable bool, blacklisted bool, whitelistable bool, whitelisted bool) (pull.Context, UpdateConfig) { +func generateUpdateTestCase(ignorable bool, ignored bool, triggerable bool, triggered bool) (pull.Context, UpdateConfig) { updateConfig := UpdateConfig{} pullCtx := pulltest.MockPullContext{} - if blacklistable { - updateConfig.Blacklist.Labels = append(updateConfig.Blacklist.Labels, "blacklist") + if ignorable { + updateConfig.Ignore.Labels = append(updateConfig.Ignore.Labels, "ignore") } - if blacklisted { - pullCtx.LabelValue = append(pullCtx.LabelValue, "blacklist") + if ignored { + pullCtx.LabelValue = append(pullCtx.LabelValue, "ignore") } - if whitelistable { - updateConfig.Whitelist.Labels = append(updateConfig.Whitelist.Labels, "whitelist") + if triggerable { + updateConfig.Trigger.Labels = append(updateConfig.Trigger.Labels, "trigger") } - if whitelisted { - pullCtx.LabelValue = append(pullCtx.LabelValue, "whitelist") + if triggered { + pullCtx.LabelValue = append(pullCtx.LabelValue, "trigger") } return &pullCtx, updateConfig diff --git a/config/bulldozer.example.yml b/config/bulldozer.example.yml index 60730c6e6..30dfc2cad 100644 --- a/config/bulldozer.example.yml +++ b/config/bulldozer.example.yml @@ -83,7 +83,7 @@ options: # # default_repository_config: # merge: - # blacklist: + # ignore: # labels: ["do not merge"] # Optional configuration to emit metrics to datadog diff --git a/config/examples/standard.bulldozer.v1.yml b/config/examples/standard.bulldozer.v1.yml index d579eda5e..8667441ec 100644 --- a/config/examples/standard.bulldozer.v1.yml +++ b/config/examples/standard.bulldozer.v1.yml @@ -1,10 +1,10 @@ version: 1 merge: - whitelist: + trigger: labels: ["merge when ready"] comment_substrings: ["==MERGE_WHEN_READY=="] - blacklist: + ignore: labels: ["do not merge"] comment_substrings: ["==DO_NOT_MERGE=="] method: squash @@ -14,5 +14,5 @@ merge: delete_after_merge: true update: - whitelist: + trigger: labels: ["wip", "update me"]