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

Remove simple-slack-notify and move to using github slack app #2819

Merged
merged 6 commits into from
Oct 19, 2023

Conversation

rmccar
Copy link
Contributor

@rmccar rmccar commented Sep 28, 2023

What is the context of this PR?

simple-slack-notify has been archived, is no longer maintained and is now running on an old version of node. This PR makes the change of removing that and then I will set up the pat-lib-notifications channel to monitor these workflows using the github slack app.
As part of this I have made the decision to split the tests out into separate workflows due to a problem in slack with the naming of the workflow containing a comma. These were separate jobs anyway that didn't rely on each other so makes sense that they are separate anyway.

Part of the fix for this ONSdigital/design-team#9

Once merged update branch protection and reconfigure slack integration

How to review this PR

  • Set up a chat between you and the github app in slack
  • Connect your github account by sending the message /github signin if not prompted
  • Send /github subscribe ONSdigital/design-system to start monitoring the repo
  • Send /github subscribe ONSdigital/design-system workflows:{name:"Lighthouse CI accessibility checks", "Publish Design System to npm", "Run visual regression tests", "Run macro and script tests"} to subscribe to the correct workflows.
  • Compare the notifications you get with the ones in the pat-lib-notifications channel and see that all notifications are replicated.
  • Unsubscribe again with the message /github unsubscribe ONSdigital/design-system

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Project for this PR (Design System) and selected the correct status
  • I have selected the correct Assignee
  • I have linked the correct Issue

@netlify
Copy link

netlify bot commented Sep 28, 2023

Deploy Preview for ons-design-system-preview ready!

Name Link
🔨 Latest commit 94a81f7
🔍 Latest deploy log https://app.netlify.com/sites/ons-design-system-preview/deploys/6530e215de731c0008cf1566
😎 Deploy Preview https://deploy-preview-2819--ons-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@rmccar rmccar added the Dependencies Pull requests that update a dependency file label Sep 28, 2023
@rmccar rmccar self-assigned this Sep 28, 2023
@rmccar rmccar changed the title Remove simple-slack-notify Remove simple-slack-notify and move to using github slack app Sep 28, 2023
@rmccar rmccar marked this pull request as ready for review September 28, 2023 11:40
Copy link
Contributor

@alessioventuriniAND alessioventuriniAND left a comment

Choose a reason for hiding this comment

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

I have followed your steps and I have noticed that I am not getting any notifications from the Macro and VR workflows.

@rmccar
Copy link
Contributor Author

rmccar commented Oct 19, 2023

I have followed your steps and I have noticed that I am not getting any notifications from the Macro and VR workflows.

Just tested it again and I seem to get the updates
Screenshot 2023-10-19 at 09 47 48
Screenshot 2023-10-19 at 09 48 41

@rmccar rmccar merged commit d58547e into main Oct 19, 2023
8 checks passed
@rmccar rmccar deleted the dependency/9/remove-simple-slack-notify branch October 19, 2023 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants