-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: Slack notifications matched on base branch name #3644
Conversation
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.
Thank you for your contribution, can you add tests for when both are set
Thank you for reviewing! I added an additional test where both are set with invalid regex. |
if err != nil { | ||
return nil, err | ||
} | ||
br, err := regexp.Compile(c.BranchRegex) |
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.
Does this become a breaking change? It might be better to use .*
when passing empty string.
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.
No, I tested a webhooks
config without defining either workspace-regex
or branch-regex
and the Slack notification still went through. For example, regexp.MatchString("", "main")
evaluates to true, so looks like this will still function fine if the user does not set the branch-regex
in the config.
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 see, thanks for your confirmation 👍
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.
LGTM
* Add ability to send slack notification matched on base branch name * Update BranchRegex usage doc * Add test with invalid branch and workspace regex
* Add ability to send slack notification matched on base branch name * Update BranchRegex usage doc * Add test with invalid branch and workspace regex
what
Add ability to send Slack notifications matched on base branch name
why
We currently use the
default
workspace for a given Atlantis project but with separate branches per environment (production
,staging
, etc). We cannot filter the Slack notifications since they all match on thedefault
workspace.tests
make test
andmake test-all
config.yaml
:Produced the following slack notification: