-
Notifications
You must be signed in to change notification settings - Fork 25
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 transpiled code loading #71
Conversation
Pull Request Test Coverage Report for Build 2016805012
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work! Can you add a tsconfig targeting esnext?
Any idea on why the tests might be flaky? I see that they are not failing in a consistent manner. |
There are likely some bugs in some parts of the code. As long as we can get a clean / full green CI run we are good. |
323b113
to
300852d
Compare
Update:
|
65326a0
to
bf1cfe8
Compare
See pinojs/pino#1243 (comment) Added test confirming that there is a real issue, and that it is corrected.
e74557d
to
d48825a
Compare
The added type declarations have been added for testing purposes, they need more love to be useful for package users.
- cover esnext code generation - use bash shell for tests in windows - wrap transpile script execution - use yarn as runner when testing yarn-pnp
d48825a
to
f84f5fc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Pull Request Test Coverage Report for Build 2016680551Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Changes
.ts
files into worker threads when usingts-node
(except in Windows).Not covered in this PR
Some potential improvements had to be left out of this PR to contain its scope:
.ts
files into worker threads when running the code in Windows platforms.Extra comments
It seems that the coverage metrics have gone down mainly because of leaving out tests on Windows for the direct loading of
.ts
files without transpilation. I'm not sure if I should explicitly document this lack of support, or if it's already enough stating it in the PR itself, as it's "just" a corner case.Related