Skip to content

Conversation

@emanuel-schmid
Copy link
Collaborator

@emanuel-schmid emanuel-schmid commented May 16, 2023

Changes proposed in this PR:

  • add a novel config value logging.climada_style that controls whether or not the logging is configured in the traditional climada style.

This PR fixes #265

PR Author Checklist

PR Reviewer Checklist

@emanuel-schmid emanuel-schmid marked this pull request as draft May 16, 2023 13:52
@emanuel-schmid emanuel-schmid changed the title Feature/unusurpate logging logging best practice May 16, 2023
@tovogt
Copy link
Collaborator

tovogt commented May 24, 2023

Thanks @emanuel-schmid, this implementation looks very clean to me!

I would be happy if somebody came up with a more fitting name for the config parameter instead of logging.climada_style but I have no strong feelings. It just seems to me like it's not about the "style" of the logging, but about whether climada is in control of the logging, or whether control of the logging is left to the user. Maybe logging.managed?

@emanuel-schmid
Copy link
Collaborator Author

Thanks @tovogt . Changed it to managed - not to close the discussion, just because it's already much better.

@emanuel-schmid emanuel-schmid requested a review from petermkr June 5, 2023 09:16
@emanuel-schmid emanuel-schmid marked this pull request as ready for review June 5, 2023 09:17
@tovogt
Copy link
Collaborator

tovogt commented Jun 6, 2023

I'm happy with this PR conceptually. Please just don't forget to update the name of the config parameter ("managed" or "climada_style" or whatever) in the CHANGELOG and in the docs, once the decision for the name is final.

@emanuel-schmid emanuel-schmid merged commit a60241a into develop Jun 6, 2023
@emanuel-schmid emanuel-schmid deleted the feature/unusurpate_logging branch June 6, 2023 16:26
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.

3 participants