search

powellnathanj / mcollective-yum-agent

http://nathanpowell.org
17 stars 11 forks source link

First attempt at a standalone mco application #8

Closed robinbowes closed 8 years ago

robinbowes commented 8 years ago

This is nowhere near finished yet, but I thought I'd share with you to get some feedback.

I wanted to add options to the check-update command and the only way to do that is to create a standalone mco application.

I'm also trying to use a common approach across as many commands as possible to reduce lines of code.

Let me know if you have questions or comments on what I've go so far.

powellnathanj commented 8 years ago

I just did a quick scan of the code changes and it looks really good. I welcome the changes. Give me a few days to do a real look and I'll happily merge it. I did see some spots (like the usage heredoc) that are place holders that would be good to finalize, but I think this many changes would be worth an actual release.

So like I said give me a few days to review, I just got back from vacation about an hour ago, but looks great.

Thank you very much for the contribution! I think it might also be time to update the author section to point to either an hash, or the README.

robinbowes commented 8 years ago

It's deffo not ready for release - but I should be able to bottom it out fairly quickly with a fresh mind :)

I'll let you know when I'm done.

R.

robinbowes commented 8 years ago

I've come back to this and reviewed the code, and made a few fixes. I'm happy that it's now ready for further/3rd-party testing.

powellnathanj commented 8 years ago

Was able to do a visual review this morning and it looks good. There is a stray comment in 'agent/yum.rb' on new line 102 '#mak'. I am going to merge these two commits and then do some testing.

I really appreciate the time you've put into this. I have wanted to revisit a proper application for some time, but work has me in a different direction. Thanks again!

robinbowes commented 8 years ago

Cheers - I'm hoping it will work as-is for our purposes, but if I find I need to make any more changes I'll send PRs and/or raise issues.

One thing I did intend to do was to deprecate mco yum downloadonly in favour of the --downloadonly switch, but you might have a view on that.