Jörg, I'm good with these patches. The diff functionality is definitely useful. I've committed them to Spacewalk master tonight.
For future commit messages, can you prefix with "spacecmd:" or something similar? Take a look at the commit history to see how Steve and I have done it. Please feel free to send other parts in logical groups of functionality. If you have a public Git repo you could always make some branches and have us pull from them; might be easier depending on how big the patch sets end up being. I'm figuring Spacewalk 1.8 will be tagged sometime in the near future, so it may not all make it into the 1.8 release. Please don't rush to get things in; if they don't make it into 1.8 we'll get them after that. I'll try my best to review in a timely fashion. /aron (Sorry for the dupe, the original got reject from -devel because this email wasn't registered) > On Mon, May 14, 2012 at 9:32 AM, Parsons, Aron <parso...@bit-sys.com> wrote: >> >> >> -----Original Message----- >> From: Jörg Steffens [mailto:joerg.steff...@dass-it.de] >> Sent: Sunday, May 13, 2012 9:03 AM >> To: Steven Hardy >> Cc: 'spacewalk-devel@redhat.com'; Parsons, Aron >> Subject: Re: [Spacewalk-devel] RFE: spacecmd: patches for printing >> >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> Hello Steve, >> >> thank you for your remarks. >> >> Am 12.05.2012 21:12, schrieb Steven Hardy: >>> 1 - Missing variable means after applying the patch, spacecmd no >>> longer works (suggest testing patches before sending in future ;) >> >> I'm so sorry. I know, I should never do last minute fixes. Fixed with >> new patch. >> >> >>> 2 - I don't get the point of >>> check_softwarechannel/is_softwarechannel they seem like unnecessary >>> wrappers IMHO >> >> yes, well, you are right. With the patches I've currently submitted, >> is_softwarechannel is only called from check_softwarechannel. >> The difference between these two functions is, that >> check_softwarechannel also generates output, while is_softwarechannel >> only performs the check and return a boolean. >> I'll need is_softwarechannel later on for automatically guessing >> softwarechannel names (find related softwarechannel). >> >> If you prefer, I can also combine these two functions into one and add >> a boolean parameter, if output should be generated or not. >> >> The same for check_activationkey, check_configchannel and check_kickstart. >> >>> 3 - You reference non-existent function >>> help_stage_configchannel_diff in do_configchannel_diff >> >> thanks. fixed. >> >> >>> 4 - You reference non-existent function >>> do_configchannel_getcorresponding in do_configchannel_diff >> >> yes, I did that on purpose. I've added a check, if this function >> exist. If not, it will not be called. >> I'm still hoping, I can add some of the functionality from my initial >> patch, at least as custom plugin. Therefore it would be very helpful, >> if this stub exists. >> Same for softwarechannel, activationkey and kickstart. >> >> I'm aware, that this might not get integrated, but I wanted to suggest >> this approach. >> >> >> >>> 5 - get_normalized_text references "stages", which is not relevant >>> now you're contributions are separated from your stage concept >> >> I've remove the word "stages", but left the example more or less >> unchanged, because in my opinion it describes best the purpose. >> Is this okay my you, or do you have other suggestions? >> >> Jörg >> - -- >> Jörg Steffens joerg.steff...@dass-it.de >> dass IT GmbH Phone: +49.221.3565666-91 >> http://www.dass-IT.de Fax: +49.221.3565666-10 >> >> Sitz der Gesellschaft: Köln | Amtsgericht Köln: HRB52500 >> Geschäftsführer: S. Dühr, M. Außendorf, Jörg Steffens, P. Storz >> -----BEGIN PGP SIGNATURE----- >> Version: GnuPG v2.0.18 (GNU/Linux) >> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ >> >> iEYEARECAAYFAk+vsRUACgkQebMQ0f2L2xt9rQCfT8SrJJ6X05F9BGWaNcAteBew >> sFEAoJy6/BzM8a0UcZZ6S8RnZon0bfSH >> =nVgZ >> -----END PGP SIGNATURE----- _______________________________________________ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
