search

jacobsalmela / NCutil

Notification Center utility- Add/remove apps, set alert styles, suppress App store notifications
GNU General Public License v2.0
171 stars 18 forks source link

Some cleanups #3

Closed gregneagle closed 9 years ago

gregneagle commented 9 years ago

I've done some clean up: previously there was a mix of tabs and spaces for indentation; now it's all consistently spaces as is recommended by most Python developers.

Second: I replaced all path construction with os.path.join() which is smarter than simply using "+"

Third: I converted all calls of external utilities to use subprocess, full paths, and not use the shell, again, all good Python style.

Hope you find these changes helpful!

jacobsalmela commented 9 years ago

I get the error:

line 31, in <module>
    ['/usr/bin/getconf', 'DARWIN_USER_DIR']).read().rstrip()
AttributeError: 'str' object has no attribute 'read'
gregneagle commented 9 years ago

That's what I get for not testing on Yosemite. subprocess.check_output returns a string, so no need for the read() call. Should be:

    darwin_user_dir = subprocess.check_output(
        ['/usr/bin/getconf', 'DARWIN_USER_DIR']).rstrip()
jacobsalmela commented 9 years ago

Cool. It works as expected now! Thanks so much! I'm going to make one more change that was bugging me when I was testing this out:

Changing the verbiage to banners and alerts (plural) as so it is consistent with the GUI wording.

gregneagle commented 9 years ago

Good! I have more PRs coming...