Skip to content
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

High Impact Alert Workflow #2346

Merged
merged 2 commits into from
May 23, 2024

Conversation

overmyheadandbody
Copy link
Contributor

This defines a new workflow that sends notifications to a Slack channel as soon as a PR with a high-impact label is opened. The previous logic from the merge-to-stage.js script has been removed, since we want such a notification to be sent out as soon as the PR is opened, not when it eventually reaches the stage branch, to ensure it gets visibility early on. Additionally, some workflows have been adapted to only run if the owner is adobecom to reduce workflow runs on forks.

Resolves: MWPW-148268

Test URLs:

Copy link
Contributor

aem-code-sync bot commented May 21, 2024

Page Scores Audits Google
/?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Contributor

@mokimo mokimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! The only use-case I'm thinking off is adding/removing labels, which would send multiple messages.

We can't easily verify a message has/hasn't already been sent though, since actions are stateless and we can't access slack.

So we need a bit of contributor education to avoid multiple messages because of adding and removing the label

.github/workflows/high-impact-alert.js Show resolved Hide resolved
.github/workflows/high-impact-alert.js Outdated Show resolved Hide resolved
@overmyheadandbody
Copy link
Contributor Author

I think most non-stage PRs get just one batch of labels, so we'll have to test this in the wild and see if it pollutes the Slack channel. The workflow could add its own label, say notification-triggered, and a PR with such a label would no longer send a notification. Except if someone manually removes it 🙂

@overmyheadandbody overmyheadandbody requested a review from a team May 22, 2024 14:48
Copy link
Contributor

Reminder to set the Ready for Stage label - to queue this to get merged to stage & production.

@narcis-radu narcis-radu self-requested a review May 23, 2024 06:40
@mokimo
Copy link
Contributor

mokimo commented May 23, 2024

I think workflow's cant be merged automatically 😕 That's just a guess though. Some PRs get this error:

Error merging 2346: High Impact Alert Workflow Resource not accessible by integration

We'll need to watch this. It sounds like a security feature so that workflows can't elevate their permissions and take over a repo

@mokimo mokimo merged commit 29d0a16 into adobecom:stage May 23, 2024
11 checks passed
@mokimo mokimo mentioned this pull request May 23, 2024
@overmyheadandbody overmyheadandbody deleted the high-impact-workflow branch August 7, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants