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

Remove istanbul #1549

Merged
merged 1 commit into from
Nov 3, 2023
Merged

Remove istanbul #1549

merged 1 commit into from
Nov 3, 2023

Conversation

koddsson
Copy link
Member

@koddsson koddsson commented Nov 2, 2023

Istanbul is deprecated and I don't think we are doing a lot with code coverage these days.

@koddsson koddsson requested a review from a team as a code owner November 2, 2023 15:45
@voxpelli
Copy link

voxpelli commented Nov 2, 2023

Replace it with nyc or c8? Modern successors to Istanbul

keithamus
keithamus previously approved these changes Nov 2, 2023
@43081j
Copy link
Contributor

43081j commented Nov 3, 2023

if we can still have coverage, that would be nice. sometimes it is useful just as a quick guide to obvious uncovered blocks, etc locally.

@voxpelli is right in that we should use c8 or nyc if we do that (i prefer c8)

maybe another PR though

@koddsson
Copy link
Member Author

koddsson commented Nov 3, 2023

if we can still have coverage, that would be nice. sometimes it is useful just as a quick guide to obvious uncovered blocks, etc locally.

@voxpelli is right in that we should use c8 or nyc if we do that (i prefer c8)

maybe another PR though

As far as I can tell the coverage doesn't seem to work at all1 and I don't see any complaints about that so I assumed that it's unused. With it not working currently I figure we remove it as non-functional and someone can add coverage back in a different PR.

If it's just used as a developer tool it might be as simple as running npx nyc mocha instead of adding it as a dev dependency of the project (that we then have to maintain and update as part of the project).

Footnotes

  1. Running npm run test-cov ends with the message: "No coverage information was collected, exit without writing coverage information"

@koddsson koddsson enabled auto-merge (squash) November 3, 2023 12:50
@koddsson koddsson disabled auto-merge November 3, 2023 12:51
@koddsson koddsson merged commit 84bd1f8 into chaijs:5.x.x Nov 3, 2023
5 checks passed
@koddsson koddsson deleted the remove-istanbul branch November 3, 2023 12:51
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.

4 participants