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
Cleanup ruby code via rubocop #85
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks lovely for the most part! let me know what you think.
spec/spec_helper.rb
Outdated
@@ -13,4 +13,5 @@ | |||
begin | |||
require 'spec_helper_local' | |||
rescue LoadError | |||
puts 'LoadError rescue' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it'd be good to have something more informative here. maybe something like this?
begin
require 'spec_helper_local'
rescue LoadError => loaderror
puts "Could not require spec_helper_local: #{loaderror.message}"
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion Eric, thanks, have added this.
spec/acceptance/nodesets/default.yml
Outdated
@@ -1,17 +1,31 @@ | |||
--- | |||
HOSTS: | |||
windows2012-64-1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modulesync is going to change this back to windows2012 because this module is now marked as cross-platform. getting around it might be hairy...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Eric, have reverted this.
54ae776
to
7823030
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from that one minor change everything looks good to me.
.travis.yml: | ||
extras: | ||
- rvm: 2.1.9 | ||
script: bundle exec rake rubocop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove this line break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There wasn't a line break. I've added one as it seems that the github diff was highlighting this as an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm strange, when I commented there was a small red arrow at the end of the line. Anyway looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No no, you're right, there was, adding a line break got rid of it :)
This commit will add code cleanup as corrected and alerted by rubocop. It also adds rules to execute rubocop on Travis.
7823030
to
5e6a0ee
Compare
This commit will add code cleanup as corrected
and alerted by rubocop. It also adds rules to
execute rubocop on Travis.