So I had a quick look:
1 - Missing variable means after applying the patch, spacecmd no longer works
(suggest testing patches before sending in future ;)
File "/home/shardy/git/spacewalk/spacecmd/src/bin/spacecmd", line 27, in
<module>
from spacecmd.shell import SpacewalkShell
File "/home/shardy/git/spacewalk/spacecmd/src/spacecmd/shell.py", line 25, in
<module>
from spacecmd.utils import *
File "/home/shardy/git/spacewalk/spacecmd/src/spacecmd/utils.py", line 664,
in <module>
def get_normalized_text( text, replacedict=None, excludes=_DIFF_EXCLUDES ):
NameError: name '_DIFF_EXCLUDES' is not defined
2 - I don't get the point of check_softwarechannel/is_softwarechannel they seem
like unnecessary wrappers IMHO
3 - You reference non-existent function help_stage_configchannel_diff in
do_configchannel_diff
if foo in in self.do_softwarechannel_list( name, True ) (which is what you do
in·
4 - You reference non-existent function do_configchannel_getcorresponding in
do_configchannel_diff
5 - get_normalized_text references "stages", which is not relevant now you're
contributions are separated from your stage concept
Ok, I stopped there - if you can do some testing and submit a revised patch
that would be good, this does look much improved generally and the features
look like they could be useful when they are working.
Steve
On Fri, May 11, 2012 at 09:35:58PM +0000, Parsons, Aron wrote:
> I have these sitting in my inbox. I'll also try to get around to looking at
> them this weekend.
>
> /aron
>
> -----Original Message-----
> Message: 1
> Date: Thu, 10 May 2012 23:06:55 +0100
> From: Steven Hardy <sha...@redhat.com>
> To: J?rg Steffens <joerg.steff...@dass-it.de>
> Cc: spacewalk-devel@redhat.com
> Subject: Re: [Spacewalk-devel] RFE: spacecmd: patches for printing
> function results and performing diff on some spacewalk components
> Message-ID: <20120510220654.ga11...@s01.fab.redhat.com>
> Content-Type: text/plain; charset=iso-8859-1
>
> On Thu, May 10, 2012 at 10:42:36PM +0200, J?rg Steffens wrote:
> > Hello,
> >
> > I've created some extensions for spacecmd to provide a way to handle
> > different stages like
> > - - development
> > - - staging
> > - - production
> >
> > This is done by following functions:
> > def do_stage_create_skel
> > def do_stage_softwarechannel_diff
> > def do_stage_softwarechannel_sync
> > def do_stage_configchannel_diff
> > def do_stage_configchannel_sync
> > def do_stage_kickstart_diff
> > def do_stage_activationkey_diff
> > def do_stage_status
> > def do_stage_dump
> >
> > As my initial approach
> > (https://bugzilla.redhat.com/show_bug.cgi?id=781883) did not make it
> > into spacecmd,
> > so now I'm now trying to add parts of the functionality as smaller
> > patches.
>
> Thanks for the patches - as I reviewed your original patch, I'm happy to
> review these (unless Aron wants to...) - I'll try to take a look tomorrow.
>
> > First, I've a question about ret
> > Most of my changes are none intrusive, however there is a behavior of
> > spacecmd I'd like to change:
> > currently, the do-functions normally have no return values at all.
> > Because some of these functions are used by other function, a lot of
> > them have an additional parameter "doreturn = False". If this is set to
> > true, the result is not printed to stdout, but instead return as list of
> > strings.
> >
> > As a results, some code inside the do-functions is duplicated.
> > Because the new functionality I've created uses some functions that
> > currently do not provide a way to return there results, I'd like to
> > suggest following approach:
> > instead of doing this differentiation inside each function, the
> > functions should always return there result as list of strings. If
> > printing is required, meaning, the function is not called from outside
> > and not from another function, the printing is done by shell.py:postcmd,
> > which will be called after all commands called from command line.
> >
> > If a function return a list of string, these will be printed to stdout.
> > If not, it behaves like before. There is no immediate need to change the
> > existing function, but of course, this would make the code cleaner.
>
> I suspect this will be quite tricky to do without breaking lots of stuff, for
> example what happens to interactive operation in your new scheme?
>
> Steve
>
>
>
> ------------------------------
>
> Message: 2
> Date: Fri, 11 May 2012 09:25:55 +0200
> From: J?rg Steffens <joerg.steff...@dass-it.de>
> To: Steven Hardy <sha...@redhat.com>
> Cc: spacewalk-devel@redhat.com
> Subject: Re: [Spacewalk-devel] RFE: spacecmd: patches for printing
> function results and performing diff on some spacewalk components
> Message-ID: <4facbf03.1080...@dass-it.de>
> Content-Type: text/plain; charset=ISO-8859-1
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Am 11.05.2012 00:06, schrieb Steven Hardy:
> [...]
> >> First, I've a question about ret Most of my changes are none
> >> intrusive, however there is a behavior of spacecmd I'd like to
> >> change: currently, the do-functions normally have no return
> >> values at all. Because some of these functions are used by other
> >> function, a lot of them have an additional parameter "doreturn =
> >> False". If this is set to true, the result is not printed to
> >> stdout, but instead return as list of strings.
> >>
> >> As a results, some code inside the do-functions is duplicated.
> >> Because the new functionality I've created uses some functions
> >> that currently do not provide a way to return there results, I'd
> >> like to suggest following approach: instead of doing this
> >> differentiation inside each function, the functions should
> >> always return there result as list of strings. If printing is
> >> required, meaning, the function is not called from outside and
> >> not from another function, the printing is done by
> >> shell.py:postcmd, which will be called after all commands called
> >> from command line.
> >>
> >> If a function return a list of string, these will be printed to
> >> stdout. If not, it behaves like before. There is no immediate
> >> need to change the existing function, but of course, this would
> >> make the code cleaner.
> >
> > I suspect this will be quite tricky to do without breaking lots of
> > stuff, for example what happens to interactive operation in your
> > new scheme?
>
> I'm not sure, what you mean be this. Normal interactive operation will
> work.
>
> 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+svwMACgkQebMQ0f2L2xv8DwCgp0NwRoiaAN3eMQ6cHFRY3TUs
> b7kAmwXFReQ48rCrmO95Zir/LPJIZSY2
> =Z8XW
> -----END PGP SIGNATURE-----
>
>
>
> ------------------------------
>
> _______________________________________________
> Spacewalk-devel mailing list
> Spacewalk-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/spacewalk-devel
>
> End of Spacewalk-devel Digest, Vol 48, Issue 5
> **********************************************
--
Steve Hardy
Professional Services Consultant, RHCE
Red Hat UK Ltd, 200 Fowler Avenue, Farnborough, Hampshire, GU14 7JP
Mobile: +44 7584 391278
Email: sha...@redhat.com
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham (USA), Mark Hegarty (Ireland), Matt Parson (USA),
Charlie Peters (USA)
_______________________________________________
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
