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

Migrate @storybook/node-logger to TypeScript #5153

Merged
merged 1 commit into from Jan 6, 2019

Conversation

dandean
Copy link
Member

@dandean dandean commented Jan 6, 2019

Issue: #5030

What I did

Migrated @storybook/node-logger to TypeScript.

Two notes:

  1. I had to do some funny stuff in the tests to get TS to allow me to call the mockReset() method. I wold love some eyes on that to see if I'm doing that correctly; there may be something I'm missing entirely.
  2. The trace() method used to pass the trace message to the first argument of npmlog.info. This was causing TS to fail to compile. To fix the issue I'm now passing the message as the 2nd argument and an empty string to the first (just as all of the other methods are doing). My guess is that passing the message as the first argument was an oversight in the first place.

How to test

Existing tests continue to work.

@dandean dandean added maintenance User-facing maintenance tasks core typescript labels Jan 6, 2019
@dandean dandean added this to the next milestone Jan 6, 2019
@dandean dandean self-assigned this Jan 6, 2019
@codecov
Copy link

codecov bot commented Jan 6, 2019

Codecov Report

Merging #5153 into next will increase coverage by 0.02%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #5153      +/-   ##
==========================================
+ Coverage   35.22%   35.24%   +0.02%     
==========================================
  Files         596      596              
  Lines        7392     7396       +4     
  Branches     1015     1010       -5     
==========================================
+ Hits         2604     2607       +3     
- Misses       4275     4276       +1     
  Partials      513      513
Impacted Files Coverage Δ
lib/node-logger/src/index.ts 80% <60%> (ø)

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 58b755c...2306a80. Read the comment docs.

@ndelangen ndelangen merged commit 7cb74db into next Jan 6, 2019
@ndelangen ndelangen deleted the ts-migration/node-logger branch January 6, 2019 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core maintenance User-facing maintenance tasks typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants