Skip to content
This repository has been archived by the owner on Mar 13, 2023. It is now read-only.

Feat: Logging alternative to #547 #548

Merged
merged 8 commits into from
Jul 4, 2022

Conversation

Kigstn
Copy link
Member

@Kigstn Kigstn commented Jun 30, 2022

What type of pull request is this?

  • Non-breaking code change
  • Breaking code change
  • Documentation change/addition
  • Tests change

Description

This is an alternative solution to #547, attempting to solve the logging problem.

This PR allows users to pass their own logger at client initialisation. The passed logger is saved in both the client and naff.constants.

This has several benefits;

  1. users don't have to do nasty stuff at the very start of their code to overwrite the naff logger before naff does anything with it
  2. custom loggers, like rich.logging or file handlers can be easily used
  3. users have more direct and easier access to the logger via both client.logger and naff.consts.logger, instead of having to logging.getLogger(logger_name) every time they want to use it
  4. naff devs have an easier time as well, since they can import the logger from anywhere as well

Changes

  • a logger obj is now defined in naff.consts which is overwritten by the logger the user passes to Client

Checklist

  • I've formatted my code with Black
  • I've ensured my code works on Python 3.10.x
  • I've tested my code

@Kigstn Kigstn added the New Feature::Library A new feature for the library label Jun 30, 2022
@LordOfPolls
Copy link
Member

On-Hold pending discussion and conflict resolution

@LordOfPolls LordOfPolls added the On-Hold This is waiting for a condition to be met before more work can be done label Jul 3, 2022
@Kigstn Kigstn removed the On-Hold This is waiting for a condition to be met before more work can be done label Jul 4, 2022
@Kigstn
Copy link
Member Author

Kigstn commented Jul 4, 2022

Ready for review

Copy link
Member

@LordOfPolls LordOfPolls left a comment

Choose a reason for hiding this comment

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

It hurts me calling it logger

Final conflict resolution
@LordOfPolls
Copy link
Member

/ok-to-test

@LordOfPolls LordOfPolls merged commit 98c431f into NAFTeam:dev Jul 4, 2022
@Kigstn
Copy link
Member Author

Kigstn commented Jul 4, 2022

It hurts me calling it logger

It hurts me calling it log

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
New Feature::Library A new feature for the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants