-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
CI: report failures on discord #19801
Conversation
6a7b73f
to
e2055b7
Compare
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.
This is awesome!
.circleci/config.yml
Outdated
command: git fetch --unshallow | ||
- list-recent-merges | ||
- discord/status: | ||
only_for_branches: chore/failure-reports |
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.
This is nice, I think if we do this only for the next
and alpha
branch, we have exactly what we want.
I first thought about only notifying on the merged
and daily
workflow, but that would also notify when you run the daily
workflow in your PR.
So setting this to `next would be better I guess.
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 had a similar thought and I think it makes sense too, I'll leave both next and alpha branches there, possibly updating to main in the future too 🤔
5ab858b
to
938f7f0
Compare
ee354cf
to
dce1f7e
Compare
982b3ed
to
4f9da2f
Compare
4f9da2f
to
89e741e
Compare
- discord/status: | ||
only_for_branches: next,alpha | ||
fail_only: true | ||
failure_message: 'Oh no! The **$CIRCLE_JOB** job has failed for **<< parameters.template >>**.\n\n**Relevant PRs of the last 24h:**\n$(git log --merges --since="24 hours ago" --pretty=format:"\`%h\` %<(12)%ar %s [%an]" | grep "Merge pull request" | sed "s/Merge pull request #/https:\/\/github.com\/storybookjs\/storybook\/pull\//g" | tr "\\n" "\\\\n" | sed "s/\\\\/\\\\n/g")' |
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.
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.
Looks great!
Two suggestions.
- Would it make sense to include in the message if this was
merged
ordaily
that failed? Would make it easier to narrow down when the issue was introduced - similarly, it might be easier to debug errors in
merged
if they only included the PR that actually got merged, and not the most recent ones.
EXCEPT if new merges cancels eachother out, I don't know. Eg. if Yann merges something to next, and I do too 3 minutes later. If my merge cancels Yann's, then an error in Yann's merge will be caught in mine, and so we would need all the recent PRs. If that is the case, disregard what I wrote.
yes they do. |
@ndelangen @JReinhold |
Issue:
What I did
Set up a reporting mechanism that will post updates to discord on workflow failures:
How to test
If your answer is yes to any of these, please make sure to include it in your PR.