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

(#22) Added support for checkupdates and apt_checkupdates #9

Merged
merged 8 commits into from
Nov 13, 2018

Conversation

KoenDierckx
Copy link
Contributor

No description provided.

@@ -108,6 +110,9 @@ def main
if ['count', 'md5'].include?(configuration[:action])
status = "%s" % [result[:data][:output]]
puts(pattern % [result[:sender], status])
elsif ['yum_checkupdates', 'apt_update', 'checkupdates', 'apt_checkupdates'].include?(configuration[:action])
status = result[:data][:outdated_packages].map{ |package| package[:package] }.join(' ')
puts(pattern % [result[:sender], status])
Copy link
Contributor

Choose a reason for hiding this comment

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

why the 2 different formats? I think this is quite nice rather than the format above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tough having more information is good, but perhaps this went a bit to far with returning all information. Updated code to return name + version in verbose mode

Copy link
Contributor

@ripienaar ripienaar left a comment

Choose a reason for hiding this comment

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

looks good apart from the syntax error, please squash the commits to one and change the commit message to (#8) Added support.....

puts(pattern % [result[:sender], result[:data][:ensure]])
elsif ['yum_checkupdates', 'apt_update', 'checkupdates', 'apt_checkupdates'].include?(configuration[:action])
status = result[:data][:outdated_packages].map{ |package| "%s-%s" % package[:package], package[:version] }.join(' ')
Copy link
Contributor

Choose a reason for hiding this comment

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

tests failing here, should be "%s-%s" % [foo, bar]

@KoenDierckx
Copy link
Contributor Author

I fixed the tests
But merging the commits has been more complicated then expected.
Git is like a swiss army knife...I keep cutting myself while playing around with it ;)

@ripienaar
Copy link
Contributor

Thanks :) I'll see if I can squash it for you, thanks a lot, will review soon

@ripienaar ripienaar changed the title Added support for checkupdates and apt_checkupdates (#22) Added support for checkupdates and apt_checkupdates Aug 22, 2018
@ripienaar ripienaar merged commit d49835a into choria-plugins:master Nov 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants