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

Next release #318

Closed
4 tasks done
coreyfarrell opened this issue Mar 5, 2019 · 7 comments
Closed
4 tasks done

Next release #318

coreyfarrell opened this issue Mar 5, 2019 · 7 comments

Comments

@coreyfarrell
Copy link
Member

coreyfarrell commented Mar 5, 2019

This issue is being created as a check-list for things to be done before the next release.

  • Implement reporting include/exclude logic with testing.
    * [ ] Other issues? @bcoe @JaKXz
    * [ ] Bump required version to node.js 8 now (see below)?

Issues to be addressed before next release can be promoted to `latest:

  • Update nyc to use TestExclude.globSync, verify this works.
  • Any other potentially breaking changes to nyc (this will be a semver-major release).
  • Verify the functionality required by @henriquemotaesteves works while honoring all edge cases of test-exclude (CC @AndrewFinlay).

@bcoe has spoken against dropping node.js 6 so the following no longer applies.


With regard to dropping node.js 6, we're likely already looking at a semver-major release for NYC. I'm wondering if we really want to have an nyc@14 semver-major bump for the include/exclude logic changes just to do another semver-major release for dropping node.js 6 shortly after? My preference is to combine these, drop node.js 6 in the next release. As of right now node.js 6 goes EOL in 27 days but it'll take some time before we're ready to release anything to npm next, then it'll take a little more time before we can promote those releases to latest.

@coreyfarrell
Copy link
Member Author

Ref istanbuljs/nyc#1010 - I think the fix does belong in NYC rather than istanbul-lib-report.

@bcoe
Copy link
Member

bcoe commented Mar 6, 2019

@coreyfarrell I'd suggest that we hold off on dropping Node@6 until 15.x. I don't want to gate our next major release on Node@6 report being dropped. Also, since nyc is used by many libraries in the ecosystem, I think it's okay if we take a tiny bit of time before dropping Node 6, to give a few other folks in the community to catch up.

Two pet features I'd love to land in our next release, if I have buy in:

  • I wonder if we should consider dropping --exclude-after-remap in favor of your idea of having a separate exclude option that applies after source-maps; my gut is this might simplify things a little bit right?
  • I would be interested in experimenting with c8 as our test runner for nyc, to avoid a build step; this need not gate landing work though.

@coreyfarrell
Copy link
Member Author

Re node@6 that's disappointing but understandable.

Right now --exclude-after-remap is a boolean (enabled by default) so it just uses the same settings as the instrumenter but does so based on the remapped filename from source-map. I'm concerned that separate test-exclude options for use after remapping would make things more complex. Raises lots of questions for me like how do defaults work? Do include/exclude settings for instrumentation apply to the include/exclude defaults for reporting? What if I set include, exclude and reporting-include but do not set reporting-exclude? Maybe --exclude-after-remap should become a set - true, false or custom? true would just use the test-exclude from the instrumenter, false would skip, custom would use a special test-exclude made for the reporter that have all of it's own options?

As for using c8 to test nyc - it would be nice to avoid the build step. I think it's important that npm t report coverage on supported versions instead of needing to use a separate npm run cover, also equally important npm t needs to function in node.js 8 (and node.js 6 I guess). So how do we setup npm t in a way that gracefully degrades for node.js < 10.12.0?

@coreyfarrell
Copy link
Member Author

Updates have been released from the monorepo to npm under the next tag. Still some work to be done before we can do the same for nyc.

CC @SimenB it'll be a bit before the updated versions are promoted to latest but I just wanted to let you know that a release is pending.

@SimenB
Copy link
Member

SimenB commented Mar 12, 2019

Thanks for the heads up! Do you think it will be breaking for Jest? I'm not sure what the breaking changes in istanbul are here. If it's some logic around include/exclude (from what I can gather from the OP), it would seem like something that won't be breaking for consumers of Jest. Might very well be wrong, though! 😀

@coreyfarrell
Copy link
Member Author

@SimenB I don't expect anything in the monorepo to be breaking. nyc will have some breaking changes but jest doesn't use that. Lots of bug fixes and code modernization. test-exclude got a few non-breaking feature options. #333 adds development *.config.js files to the default exclude list including jest.config.js.

@coreyfarrell
Copy link
Member Author

New releases are now latest on npm for all monorepo modules, nyc and babel-plugin-istanbul.

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

No branches or pull requests

3 participants