-
Notifications
You must be signed in to change notification settings - Fork 51
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
Use stdout.isTTY instead of tty.isatty #103
Use stdout.isTTY instead of tty.isatty #103
Conversation
Hey, @jorgebucaran, can you please take a look at this PR?! 👀 |
Do you think we can fix the failing tests? 🤔 |
I will take a look, but the failing tests seem to not be related to this PR 🤔 |
I tried locally with different versions of node 10/12/14, and it seems the problem can be in the incompatibility of the libraries. For example, we try to run Rollup on Node v10, but the Rollup minimum Node version is 18 You would have to use Rollup v2.9.1 (2 years ago) to make it work for all node versions probably |
Thank you! Yes, we don't really want to do that, so will need to update our infra a bit. |
Hey, @jorgebucaran pushed a commit here to test if building with Rollup v2 works. |
package.json
Outdated
"deploy": "npm test && git commit --all --message $tag && git tag --sign $tag --message $tag && git push && git push --tags", | ||
"release": "tag=$npm_package_version npm run deploy && npm publish --access public", | ||
"prepare": "npm run build" | ||
}, | ||
"devDependencies": { | ||
"c8": "*", | ||
"rollup": "^2.79.1", |
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.
We don't need to list rollup if we're using npx, do we?
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.
npx pulls the package from the registry if it is not found locally.
We can run it directly for smaller package download sizes.
I'm pushing another change.
6a79eaf
to
6525e21
Compare
Closes #100