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

[Petr Spacek]

On 4.10.2014 01:58, Gabe Alford wrote:
> Thanks Petr. Updated patch attached.

Petr^3, is it okay now?

Petr^2 Spacek

>
> 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.