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

feat: make log as protected #407

Merged
merged 1 commit into from
Mar 23, 2023
Merged

feat: make log as protected #407

merged 1 commit into from
Mar 23, 2023

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Feb 22, 2023

Motivation
Lodestar wants to incorporate gossipsub log to its own log files

Description

  • According to https://github.com/debug-js/debug#output-streams we should be able to configure per-namespace by overriding the log method, however when I tried in lodestar it does not work and we have to use the same log instance
  • Change log from private to protected so that child class can access and redirect to its own log

Closes #406

@twoeths twoeths requested a review from a team as a code owner February 22, 2023 03:34
@what-the-diff
Copy link

what-the-diff bot commented Feb 22, 2023

  • Add a new property log to the class.
  • Make it protected so child classes can override and redirect logs to their own loggers if needed.
  • Remove unused variable declaration of directPeerInitial in constructor, since we are not using this anymore after refactoring codebase for v1 protocol implementation (https://github.com/libp2p/js-libp2p-gossipsub/pull/534).

@codecov-commenter
Copy link

Codecov Report

Base: 83.31% // Head: 83.29% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (1903dc3) compared to base (1e1cf5a).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #407      +/-   ##
==========================================
- Coverage   83.31%   83.29%   -0.02%     
==========================================
  Files          48       48              
  Lines       11800    11804       +4     
  Branches     1271     1270       -1     
==========================================
+ Hits         9831     9832       +1     
- Misses       1969     1972       +3     
Impacted Files Coverage Δ
src/index.ts 70.40% <100.00%> (-0.07%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@wemeetagain wemeetagain merged commit 5b3aee9 into master Mar 23, 2023
@wemeetagain wemeetagain deleted the tuyen/protected_log branch March 23, 2023 13:59
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.

Add logger option
4 participants