Skip to content
This repository has been archived by the owner on Jan 30, 2024. It is now read-only.

Enable email notifications for the Jenkins Smokey job #5825

Merged
merged 1 commit into from
Apr 20, 2017

Conversation

chrisroos
Copy link
Contributor

I'd like to know as soon as possible if a change we've deployed means
that an associated Smokey test fails.

This is the same as the change I made for the Deploy App Jenkins job in
4cc8844.

Prior to this change the only notification mechanism was Slack. This was
disabled in integration (the Slack auth token is set to disabled for environment in /etc/jenkins_jobs/jobs/smokey.yaml on CI) meaning that
we'd only see failures by looking at the Jenkins interface. Although it
was enabled in staging and production, the Slack notification only
includes a link to the failure which I'm not able to view because I
don't have access to those Jenkins instances.

I've copied the email configuration from the deploy_app.yaml.erb file as
I know we've received email notifications from that job in Jenkins.

I haven't added a test for this change as we don't appear to have added
tests for similar changes in the past.

I'd like to know as soon as possible if a change we've deployed means
that an associated Smokey test fails.

This is the same as the change I made for the Deploy App Jenkins job in
4cc8844.

Prior to this change the only notification mechanism was Slack. This was
disabled in integration (the Slack auth token is set to `disabled for
environment` in /etc/jenkins_jobs/jobs/smokey.yaml on CI) meaning that
we'd only see failures by looking at the Jenkins interface. Although it
was enabled in staging and production, the Slack notification only
includes a link to the failure which I'm not able to view because I
don't have access to those Jenkins instances.

I've copied the email configuration from the deploy_app.yaml.erb file as
I know we've received email notifications from that job in Jenkins.

I haven't added a test for this change as we don't appear to have added
tests for similar changes in the past.
@floehopper
Copy link
Contributor

I'm not sure whether I'm allowed to approve this, but the change looks good to me. 👍

@chrisroos
Copy link
Contributor Author

I've asked in Slack whether you're allowed to approve this PR, @floehopper:

Oliver Byford
[5:19 PM]
the reviewer has to have write access to the repo

[5:19]
for it to ‘count’ with required reviews

Chris Roos [5:20 PM]
And is that the only requirement?

Oliver Byford
[5:21 PM]
“There are just four rules of reviewing and merging PRs:
master must be able to be released at any time
The change must have two reviews from people from GDS (preferably GOV.UK). This can (and normally will) include the author.
Use the Github Review UI to mark a PR as approved or requiring changes.
Use the Github UI to merge the PR. This ensures the PR number is added to the merge commit.”

[5:21]
from https://gov-uk.atlassian.net/wiki/display/GOVUK/RFC+52+-+Pull+Request+merging+process – which you might not be able to access…

[5:21]
so I would say as long as they’re confident in the change, I don’t see why not…

Chris Roos [5:22 PM]
Awesome. That’s really useful. Thanks @obyford 🙂

In summary: you're allowed but it's up to you to decide whether you're confident in the change to do so.

Copy link
Contributor

@floehopper floehopper left a comment

Choose a reason for hiding this comment

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

👍

@chrisroos
Copy link
Contributor Author

Splendid. Thanks @floehopper. I'll get this merged now.

@chrisroos chrisroos merged commit b2e6ac7 into master Apr 20, 2017
@chrisroos chrisroos deleted the enable-email-notifications-for-smokey-jenkins-job branch April 20, 2017 16:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants