Skip to content

Conversation

stevenvdb
Copy link

@stevenvdb stevenvdb commented Sep 23, 2025

In most places, python-irodsclient uses named loggers (e.g., logger = logging.getLogger(__name__); logger.debug(...)). This is nice because it allows to separate logging done by the Python code using the irodsclient from the logging of irodsclient itself. There were however still a few cases where the root logger was used and this PR corrects that.

@trel
Copy link
Member

trel commented Sep 23, 2025

oh, very nice. thank you. we'll get an issue created and linked.

Copy link
Collaborator

@d-w-moore d-w-moore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good! I've made an issue for this, #771, so @stevenvdb if you could link to that issue into the commit message, (we usually just prepend a string like [_771] for instance, referencing the issue) I think we can approve it fairly quickly.

@stevenvdb stevenvdb changed the title Use named loggers instead of directly calling logging [#771] Use named loggers instead of directly calling logging Sep 23, 2025
@stevenvdb stevenvdb force-pushed the use_named_loggers branch 2 times, most recently from fa56b23 to f2f18dd Compare September 23, 2025 16:56
Copy link
Contributor

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@d-w-moore Awaiting your approval.

@korydraughn
Copy link
Contributor

@stevenvdb Please update the commit message to use a # instead of _. For instance:

[#771] Use named loggers instead of directly calling logging

Thanks for your contribution!

@alanking
Copy link
Contributor

@stevenvdb - I can see a commit with the updated commit message here: stevenvdb@f2f18dd But that commit does not appear to be on any branch, or at least it's not on the branch for this PR. Can you please push that commit to the use_named_loggers branch in your fork so that this PR is updated? Then, we can merge this. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants