On Friday 09 of March 2012 13:31:18 Duncan Mac-Vicar P. wrote: > When I was looking into making the channel "rhn" check prefix optional, > I realized the checks where duplicated: Channel creation was done with > CreateChannelCommand and cloning with a NewChannelHelper. > > The attached patch does the following: > > - Moves the clone channel logic from NewChannelHelper to > CloneChannelCommand (new class) > - Moves the common stuff (data members) shared by > (Create/Clone)ChannelCommand to a base class BaseNewChannelCommand. Also > makes members private and use getters instead. > - Factors out the common validation from both actions into > ChannelHelper, used now by both commands. > - Testcase for the ChannelHelper (Haven't been able to run it yet) > - Update the dependent code in ChannelSoftwareHandler and > UpdateChannelCommand. > > Please review the patch. I would like to know if it goes in the right > direction so I can test it more and improve it. > > In my own testing I started to get a Internal Server Error when conning > a channel and manually setting the label/name back to the original ones > (removing "Clone of"). Because the crash did not happen in our internal > installation I assumed it was my fault, but then I realized it happens > also in our QA machine with Postgres with the current 1.7 code. It > happens in the .pxt pages for cloning when on postgres. Which means this > channel validation code was not duplicated but triplicated. For the perl > part the best solution is to rewrite that page in Java later. Should I > open a bugreport for the Internal Server Error? > > Duncan
Hey Duncan, the patch is basicaly ok. It just didn't pass out checkstyle rules. See http://pastebin.com/5Uw7euZj or run 'ant checkstyle' in the spacewalk/java directory. Please resolve them and send the patch in the git patch format - using 'git format-patch'. Thank you, -- Tomas Lestach RHN Satellite Engineering, Red Hat _______________________________________________ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
