Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some logging inefficiencies #1515

Merged
merged 3 commits into from Jul 6, 2023

Conversation

semaperepelitsa
Copy link
Contributor

When reviewing Faraday logger I noticed two inefficiencies:

  • It validates :log_level option on every log message, when it could only do it once during initialisation.
  • It allocates a Proc object on every log message for no reason.

This PR fixes both issues.

Copy link
Member

@iMacTia iMacTia left a comment

Choose a reason for hiding this comment

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

Thank you @semaperepelitsa for addressing these performance issues 🙌 !

This LGTM, we just have a minor rubocop offense.
The suggested fix from Rubocop works, but it's not ideal in this particular case, so if you prefer you could try moving @filter initialisation after @options and that should also solve the offense 👍

@semaperepelitsa
Copy link
Contributor Author

@iMacTia, done!

Related PR: #1079

cc @amrrbakry

@iMacTia iMacTia merged commit d1743e6 into lostisland:main Jul 6, 2023
8 checks passed
@semaperepelitsa semaperepelitsa deleted the logger-tweaks branch July 6, 2023 10:03
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.

None yet

2 participants