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

Alert users when skipping review #1206

Merged
merged 1 commit into from Feb 13, 2020
Merged

Conversation

@beccapearce
Copy link
Contributor

beccapearce commented Feb 13, 2020

To reduce the possibility of force publishing without anyone noticing we have added:

  • Warning text to the skip review dialog box
  • An email to be sent out when review is skipped

Trello card: https://trello.com/c/jUuJwRLW

app/mailers/noisy_workflow.rb Outdated Show resolved Hide resolved
@issyl0
issyl0 approved these changes Feb 13, 2020
Copy link
Member

issyl0 left a comment

LGTM apart from one tiny comment about the message wording.

@cbaines

This comment has been minimized.

Copy link
Contributor

cbaines commented Feb 13, 2020

To reduce the possibility of force publishing without anyone noticing we have added:

* Warning text to the skip review dialog box
* An email to be sent out when review is skipped

This information would be useful to have in the commit message.

@beccapearce beccapearce force-pushed the alert-when-force-publishing branch from 6554e40 to 82fe03f Feb 13, 2020
Copy link
Contributor

cbaines left a comment

Looks good to me 👍

Reduce the chance of force publishing without anyone noticing by:
- Adding warning text to the skip review dialog box
- Creating an email alert when review is skipped
@beccapearce beccapearce force-pushed the alert-when-force-publishing branch from 82fe03f to 3eb8abc Feb 13, 2020
@beccapearce beccapearce merged commit 97a1cb3 into master Feb 13, 2020
3 checks passed
3 checks passed
continuous-integration/jenkins/branch This commit looks good
Details
continuous-integration/jenkins/publishing-e2e-tests Publishing end-to-end tests succeeded on Jenkins
Details
continuous-integration/jenkins/security No security issues found
Details
@beccapearce beccapearce deleted the alert-when-force-publishing branch Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.