Skip to content
This repository was archived by the owner on Oct 3, 2023. It is now read-only.

Simplify Logger interface, remove silly, make level optional#271

Merged
mayurkale22 merged 3 commits intomasterfrom
simplify-console-interface
Jan 10, 2019
Merged

Simplify Logger interface, remove silly, make level optional#271
mayurkale22 merged 3 commits intomasterfrom
simplify-console-interface

Conversation

@draffensperger
Copy link
Copy Markdown
Contributor

By removing silly and making the level attribute optional (and defaulted to debug) then the console interface itself will implement Logger. See this TS playground

This will be useful for opencensus-web where a major goal is small code size for web clients, so being able to just use console itself as a logger cuts down on code size.

This does seem like a relatively minor breaking change though.

Copy link
Copy Markdown
Contributor

@justindsmith justindsmith left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@mayurkale22 mayurkale22 left a comment

Choose a reason for hiding this comment

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

Can you please update CHANGELOG with breaking change comment?

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #271 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #271      +/-   ##
=========================================
+ Coverage    94.8%   94.8%   +<.01%     
=========================================
  Files         110     110              
  Lines        7779    7764      -15     
  Branches      713     713              
=========================================
- Hits         7375    7361      -14     
+ Misses        404     403       -1
Impacted Files Coverage Δ
src/common/console-logger.ts 93.1% <0%> (-0.45%) ⬇️
test/test-console-logger.ts 100% <0%> (ø) ⬆️
test/test-stackdriver-monitoring.ts 97.36% <0%> (+0.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3db8bf5...5fa730d. Read the comment docs.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 10, 2019

Codecov Report

Merging #271 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #271      +/-   ##
=========================================
+ Coverage    94.8%   94.8%   +<.01%     
=========================================
  Files         110     110              
  Lines        7779    7764      -15     
  Branches      713     713              
=========================================
- Hits         7375    7361      -14     
+ Misses        404     403       -1
Impacted Files Coverage Δ
src/common/console-logger.ts 93.1% <0%> (-0.45%) ⬇️
test/test-console-logger.ts 100% <0%> (ø) ⬆️
test/test-stackdriver-monitoring.ts 97.36% <0%> (+0.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3db8bf5...991e456. Read the comment docs.

@draffensperger
Copy link
Copy Markdown
Contributor Author

@mayurkale22 I added a note to the changlog, could you take another look and merge if all is good?

@mayurkale22 mayurkale22 merged commit 7af93bb into master Jan 10, 2019
@mayurkale22 mayurkale22 deleted the simplify-console-interface branch January 10, 2019 22:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants