Code review comment for lp://staging/~mvo/software-center/cleanup-in-config-code

Revision history for this message
Gary Lasker (gary-lasker) wrote :

Hi again Michael. So I noticed that there are some pyflakes warnings in this branch:

$ pyflakes .
./tests/test_config.py:5: 'patch' imported but unused
./softwarecenter/config.py:79: redefinition of function 'add_to_unity_launcher' from line 71

The first is trivial, but I wanted to ask you to take a look at the second to decide how you would like to best address this, as we'll use this pattern for the other config values as well.

I was also thinking that it would probably be better to make this change for all of the config values in a single branch rather than piecemeal (as you mentioned, we should do this for "recommender_profile_id", "uuid", "accepted_tos", "size", "maximized"). That said, I'm also happy to approve/merge this for add_to_unity_launcher only (once the pyflakes issue is addressed) if you prefer that, as the goal as always is "incremental improvement" as opposed to "perfection".

There are also some pep8 issues here, but they are trivial to fix as well.

Thanks again, Michael!

review: Needs Fixing

« Back to merge proposal