In general, patch looks good. 1. Are the parens a PEP8/ pycharm/ pylint thing? If not, I'm not sure I see the point of adding them. 2. profile.py needs the standard RH copyright notice at the top. 3. I'd like to see a little more testing in the main(). In particular, I'd like to see the use of Profile Outputs and ProfilePolicySets. Ade On Wed, 2014-06-18 at 17:50 +1000, Fraser Tweedale wrote:
On Wed, Jun 18, 2014 at 01:17:23AM -0400, Abhishek Koneru wrote:
Please review the patch which implements the remaining of the ProfileClient API for create/modify/delete profiles.
Includes basic test cases in the main method of profile.py module.
-- Abhishek
Applied the patch and ran the main method to test. Does what it says on the tin, but haven't rigorously tested it / used it in anger yet.
Patch looks fine, just one nitpick (see below).
Cheers,
Fraser
diff --git a/base/common/python/pki/__init__.py b/base/common/python/pki/__init__.py index 891d6ea6364b037f132ff3754b73b372c638b0f7..e9b726cf763785b4a700ef314ff27774b13aba40 100644 --- a/base/common/python/pki/__init__.py +++ b/base/common/python/pki/__init__.py @@ -168,7 +168,7 @@ class PKIException(Exception, ResourceMessage): ret = cls(json_value['Message'], json_value['Code'], json_value['ClassName']) for attr in json_value['Attributes']['Attribute']: - print str(attr) + print(str(attr)) ret.add_attribute(attr["name"], attr["value"]) return ret
@@ -299,7 +299,7 @@ class PropertyFile(object): def show(self): """ Show contents of property file.""" for line in self.lines: - print line + print(line)
Nit-pick: Why the parens? Even if this is the correct style, it is unrelated to the other changes so might be better to include in a separate patch?
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel