-
Notifications
You must be signed in to change notification settings - Fork 280
Fix macOS home-brew PR slack notifications for new release #7102
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
Fix macOS home-brew PR slack notifications for new release #7102
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #7102 +/- ##
========================================
Coverage 77.87% 77.87%
========================================
Files 1576 1576
Lines 181568 181588 +20
========================================
+ Hits 141394 141413 +19
- Misses 40174 40175 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
scripts/slack_notification_action.go
Outdated
case "failure": | ||
color = "danger" | ||
default: | ||
color = envOr(EnvSlackColor, "good") |
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.
Should the default really be "good"? Isn't this what we would see if we have some unexpected outcome (infrastructure unavailable?)... I would assume we'd like to see some warning or error, not that it worked.
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'll see if I can find one that's a neutral colour (say, yellow or grey)
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.
Or failure. We should assume the "bad" path here, not the "good" path.
scripts/slack_notification_action.go
Outdated
AuthorName: envOr(EnvGithubActor, ""), | ||
AuthorLink: os.Getenv("GITHUB_SERVER_URL") + "/" + os.Getenv(EnvGithubActor), | ||
AuthorIcon: os.Getenv("GITHUB_SERVER_URL") + "/" + os.Getenv(EnvGithubActor) + ".png?size=32", | ||
Footer: envOr(EnvSlackFooter, "<https://github.com/rtCamp/github-actions-library|Powered By rtCamp's GitHub Actions Library>"), |
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.
Can we do this without a footer? If not, can we make the footer empty? I don't see a benefit in advertising/adding footers here. (The messages are already quite spammy...)
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.
Yeah, happy to leave this empty.
Currently, the slack notification action we use to notify
us in case jobs fail doesn't work on macOS and Windows (because
of a docker issue bootstrapping an Alpine linux environment on
top of those, see rtCamp/action-slack-notify#22).
This is allowing us to do that, by cloning the action source code
into our
scripts/
folder, and then running it using a downloadedgolang
toolchain - instead of needing to run the whole action ontop of docker.