Review: Needs Fixing

A few comments / requested fixes:
 - using 'passwd -l' is ok globally I think (rather than copying that)
   just put a comment in the parent class that the long format is not
   supported on suse. 'chpasswd -e' is ok too.
 
   I do prefer the long command names (as just now I had to look up
   what '-e' meant), but it reduces code duplication here.  Just add
   some comments.

   also, please leave in the 'lock_passwd' as that makes sense to
   be its own method i think.

 - "Copyright (C) 2013 SUSE LLC"
   We'll need the contributors agreement signed by the authoring party.
   http://www.canonical.com/contributors

 - '--no-gpg-checks'
   If 'zypper -t' is analagus to 'apt-get update', then you ignoring gpg
   checks is dangerous, as the content is pulled over non-secure transport
   so without a gpg check you're vulnerable to man in the middle.

   If the image creator has added additional repositories that they trust
   then they most certainly should have imported the gpg keys for those
   repositories as well.

   Did I miss something?


 - can we get packages/rpmb to work on suse? or should would it be better to
   do packages/suse-rpmb ?

Those are moderately small things.
The big thing is thank you for your contribution.

Scott


-- 
https://code.launchpad.net/~juergh/cloud-init/add-sles-support/+merge/171223
Your team cloud init development team is requested to review the proposed merge 
of lp:~juergh/cloud-init/add-sles-support into lp:cloud-init.

_______________________________________________
Mailing list: https://launchpad.net/~cloud-init-dev
Post to     : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to