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

[Petr Viktorin]

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 --------------
A non-text attachment was scrubbed...
Name: freeipa-rga-33+pviktori-Remove-trivial-path-constants-from-modules.patch
Type: text/x-patch
Size: 51354 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140930/30414151/attachment.bin>