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

Ops 508 - Send notifications of codebuild job status changes. #1

Merged
merged 10 commits into from
Dec 5, 2018

Conversation

alboyadjian
Copy link
Member

No description provided.

@alboyadjian
Copy link
Member Author

@davexunit Do you think it's worth breaking this up from a big-ass procedural script to ruby classes right now? Or should we wait till we've run it in the wild a little?

end

def secrets_manager_slack_secret_name
ENV['CB_NOTIFIER_SLACK_SECRET_NAME'] || 'slack/code-build'

Choose a reason for hiding this comment

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

Style nit: "codebuild", one word, no dash.

table_name: dynamo_table_name
)

unless old_status != current_status

Choose a reason for hiding this comment

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

Replace double negative: if old_status == current_status

author_name = git_info[1]
author_email = git_info[2]
committer_email = git_info[3]
commit_subject = git_info[4]

Choose a reason for hiding this comment

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

short_hash, author_name, author_email, committer_email, commit_subject = git_info

unless lookup_response.ok
cb_puts "Slack user lookup by email for #{email} failed with " \
"error: #{lookup_response.error}"
next

Choose a reason for hiding this comment

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

Instead of using next, use if/else and incorporate the rest of the code after the unless block.

@alboyadjian alboyadjian merged commit 8332947 into master Dec 5, 2018
@alboyadjian alboyadjian deleted the ops-508 branch December 5, 2018 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants