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(deps): replace cover/istanbul with nyc #1823

Merged
merged 1 commit into from
Apr 1, 2020

Conversation

mmarchini
Copy link
Contributor

istanbul is not supported anymore, and the last version of cover was
released seven years ago. cover also depends on a vulnerable package
(which makes npm audit noisy). Since make cover was not working
anymore, replacing it with nyc (istanbul's successor) should be
straightforward.

Signed-off-by: Matheus Marchini [email protected]

Pre-Submission Checklist

  • Ran the linter and tests via make prepush

`istanbul` is not supported anymore, and the last version of `cover` was
released seven years ago. `cover` also depends on a vulnerable package
(which makes `npm audit` noisy). Since `make cover` was not working
anymore, replacing it with `nyc` (`istanbul`'s successor) should be
straightforward.

Signed-off-by: Matheus Marchini <[email protected]>
@retrohacker retrohacker merged commit 361f83e into restify:master Apr 1, 2020
@wesleytodd
Copy link
Member

I only ended up here because of the release please PR, so sorry if this is bringing up something unnecessarily, but have you considered using c8? I have started using it on a few projects and it is working great for me.

https://www.npmjs.com/package/c8

@mmarchini
Copy link
Contributor Author

Haven't looked into it yet. What made you change? How hard was the change?

I think nyc is working for us right now, but I'm open to changing if it becomes a problem at some point.

@wesleytodd
Copy link
Member

It was super simple to change, just swap in c8 where you call nyc. The nice thing about c8 is that it uses v8's internal coverage api. Maybe @bcoe could elaborate on other benefits?

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.

3 participants