On 8/11/2015 2:31 PM, Christian Heimes wrote:
Large portions of the patch was automatically created with autopep8:
find base/ -name '*.py' | xargs autopep8 --in-place --ignore E309 \ --aggressive find base/common/upgrade base/server/upgrade -type f -and \ -not -name .gitignore | autopep8 --in-place --ignore E309 --aggressive autopep8 --in-place --ignore E309 --aggressive \ base/common/sbin/pki-upgrade \ base/server/sbin/pkispawn \ base/server/sbin/pkidestroy \ base/server/sbin/pki-server \ base/server/sbin/pki-server-upgrade
About two dozent violations were fixed manually.
https://fedorahosted.org/pki/ticket/708
NOTE: I'm going to add some checks to the PKI core spec file and tox later.
Some comments: 1. In base/common/python/pki/cli.py we probably shouldn't insert a line break inside an argument: # If module command exists, include it as arguments: <module # command> <args>... 2. In base/common/python/pki/crypto.py it probably should have been a decorator, but this is a separate issue. # abc.abstractmethod def get_cert(self, cert_nick): 3. In base/kra/functional/drmclient_deprecated.py this code is probably less readable than the original. Is there a better way to format it? It's a deprecated code too, maybe we can just ignore/remove it. response['cert'] = b64.replace( CERT_HEADER, "").replace( CERT_FOOTER, "") 4. In base/server/python/pki/server/cli/migrate.py the line break isn't very nice: if valve.get( 'className') == 'org.apache.catalina.valves.AccessLogValve': Everything else looks good. It's up to you how you want to address the above items. Regardless, it's ACKed. -- Endi S. Dewata