[Freeipa-devel] [PATCH 0033] Remove trivial path constants

[Gabe Alford]

Thanks Petr. Updated patch attached.

On Tue, Sep 30, 2014 at 10:59 AM, Petr Viktorin <pviktori at redhat.com> wrote:

> On 09/30/2014 05:13 AM, Gabe Alford wrote:
>
>> Updated patch to fix merge conflicts from recent changes.
>>
>> On Wed, Sep 24, 2014 at 8:34 PM, Gabe Alford <redhatrises at gmail.com
>> <mailto:redhatrises at gmail.com>> wrote:
>>
>>     Hello,
>>
>>     Patch for https://fedorahosted.org/freeipa/ticket/4399. Let me know
>>     if I missed any.
>>
>>     Thanks,
>>
>>     Gabe
>>
>
> Thanks for the patch, and sorry for the delay!
>
> ipaserver/tools/ipa-upgradeconfig:
> The `filename` and `ca_file` aren't module-level constants; I think in
> this case they improve readability.
> The ticket calls for removing module-level lines like:
>     NSSWITCH_CONF = paths.NSSWITCH_CONF
> which are just silly, but assigning a name locally to a global constant is
> a valid thing to do -- even if the local name just says "the file we're
> working on now".
>
>
>  -        ca_file = paths.ALIAS_CACERT_ASC
>> -        if os.path.exists(ca_file):
>> +        if os.path.exists(paths.SYSCONFIG_HTTPD):
>>
>
> Whoops!
>
>
> install/wsgi/plugins.py:
>
>  -PLUGINS_DIR = paths.IPA_JS_PLUGINS_DIR
>> -
>>
> [...]
>
>> -    if not os.path.isdir(PLUGINS_DIR):
>> +    if not os.path.isdir(paths.IPA_CA_CSR):
>>
>
> Whoops too!
>
>
> ipaplatform/fedora/tasks.py:
> ipa-client/ipa-install/ipa-client-install:
> ipaserver/install/dsinstance.py:
> ipaserver/install/httpinstance.py:
> Again, I'd not change the target_fname, filepath.
>
>
> ipapython/sysrestore.py:
> Again, `SYSRESTORE_PATH` describes better what we do with `paths.TMP`, so
> I'd prefer keeping it.
>
>
> ipaserver/install/adtrustinstance.py:
> I don't think we want to convert the self.* to constants.
>
>
> ipaserver/install/certs.py:
> I'd leave NSS_DIR as it is, rather than lose the comment.
>
>
> ipapython/ipautil.py:
> ipaserver/install/ldapupdate.py:
> ipalib/session.py:
> ipaserver/install/bindinstance.py:
> SHARE_DIR, UPDATES_DIR, and krbccache_dir, NAMED_CONF (respectively) need
> to stay, unless you also replace them in everything that uses them.
>
> Be sure to run make-lint after doing these changes.
>
>
> I've rebased, and I made some of the changes as I went along the review.
> You can base another revision on the attached patch.
>
> --
> Petr³
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141003/2837cb1d/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rga-0033-3-Remove-trivial-path-constants-from-modules.patch
Type: text/x-patch
Size: 18537 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141003/2837cb1d/attachment.bin>