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

Implement notifications configuration for .travis.yml #136

Merged
merged 3 commits into from
Nov 27, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -88,6 +88,8 @@ Travis uses a .travis.yml file in the root of your repository to learn about you
|branches |Allows you to specify the only branches that travis will run builds on. The default branches are `master` and `/^v\d/`. |
|branches_except |Allows you to specify branches that travis will not build on.|
|remove_branches |Allows you to remove default branches set in config_defaults.yml.|
|notifications |Allows you to specify the notifications configuration in the .travis.yml file.|
|remove_notifications |Allows you to remove default branches set in config_defaults.yml.|

### .yardopts

2 changes: 2 additions & 0 deletions config_defaults.yml
Original file line number Diff line number Diff line change
@@ -61,6 +61,8 @@
branches:
- master
- /^v\d/
notifications:
email: false
.yardopts:
markup: markdown
appveyor.yml:
13 changes: 12 additions & 1 deletion moduleroot/.travis.yml.erb
Original file line number Diff line number Diff line change
@@ -88,7 +88,18 @@ branches:
<% end -%>
<% end -%>
notifications:
email: false
<% notifications_arr = ((@configs['notifications'].to_a || []) - (@configs['remove_notifications'].to_a || [])) -%>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to guard against @configs['notifications'] possibly being nil? I realize it has a default, but someone maybe could override it to nil?

Copy link
Contributor

Choose a reason for hiding this comment

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

Was there a situation where this value was not an array but could be safely coerced into an array? If not, we can just drop to:

Suggested change
<% notifications_arr = ((@configs['notifications'].to_a || []) - (@configs['remove_notifications'].to_a || [])) -%>
<% notifications_arr = ((@configs['notifications'] || []) - (@configs['remove_notifications'] || [])) -%>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scotje Notifications is a hash, but in order to provide a "difference" between the @config['notifications'] and @config['remove_notifications'] i converted to an array to make this difference logic easier.

<% unless notifications_arr.empty? -%>
<% notifications = Hash[*notifications_arr.flatten] -%>
<% notifications.keys.sort.each do |key| -%>
<% if notifications[key].is_a?(Array) or notifications[key].is_a?(Hash) -%>
<%= key %>:
<%= notifications[key].to_yaml.sub(/---\R/, '').gsub(/\R/, "\n ").strip %>
<% else -%>
<%= key %>: <%= notifications[key] %>
<% end -%>
<% end -%>
<% end -%>
<% if @configs['deploy'] -%>
deploy:
provider: puppetforge