Skip to content

Fix so that the logger only returns one global logger, allowing logging#149

Closed
koldinger wants to merge 1 commit intoicloud-photos-downloader:masterfrom
koldinger:master
Closed

Fix so that the logger only returns one global logger, allowing logging#149
koldinger wants to merge 1 commit intoicloud-photos-downloader:masterfrom
koldinger:master

Conversation

@koldinger
Copy link
Copy Markdown

settings to be used globally, and eliminating the overlogging when a log-level is specified.

@iambenmitchell
Copy link
Copy Markdown

How did you get it working? I have an issue with albums, #150

@koldinger
Copy link
Copy Markdown
Author

Can't imagine that this is related. This merely uses a common logging object, so that the settings get reflected everywhere, rather than a separate logging object for each component.

@iambenmitchell
Copy link
Copy Markdown

I understand, just asking how you got your build to work, since the master is broken

@koldinger
Copy link
Copy Markdown
Author

I just ran it. Seems to be fine.

@ndbroadbent
Copy link
Copy Markdown
Collaborator

Hi @koldinger, thanks very much for the PR, and sorry for the delay! I was just wondering if you could please take a look at the pylint errors in the Travis CI build? (If you are still interested)

3.73s$ pylint icloudpd
************* Module icloudpd.logger
icloudpd/logger.py:35:0: C0103: Constant name "logger" doesn't conform to UPPER_CASE naming style (invalid-name)
icloudpd/logger.py:38:4: C0103: Constant name "logger" doesn't conform to UPPER_CASE naming style (invalid-name)
icloudpd/logger.py:38:4: W0603: Using the global statement (global-statement)
icloudpd/logger.py:39:4: R1705: Unnecessary "else" after "return" (no-else-return)

@menkej
Copy link
Copy Markdown
Collaborator

menkej commented Oct 11, 2020

I like using a singleton for logging, so I understand the intention. Anyways pylint has a good point not to use globals, etc. But to solve this we should restructure the program in a more object-oriented way (to be a class), so that we could just put the logger into an instance variable. But this is quite a big change...

@menkej
Copy link
Copy Markdown
Collaborator

menkej commented Nov 1, 2020

Thanks again for your pull request. We added a different solution with #194

@menkej menkej closed this Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants