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: add delete option to instrument command #1005

Merged
merged 13 commits into from Mar 12, 2019

Conversation

AndrewFinlay
Copy link
Contributor

@AndrewFinlay AndrewFinlay commented Feb 27, 2019

This change adds a --clean switch to the nyc instrument command, it must be set true and an output directory must be specified.

I've set the default value for this to false so it can be a non-breaking change, but this should change in the next major release. I've included tests for this function, introduced lint support for afterEach in the tests, and required the makeDir module.

AndrewFinlay and others added 9 commits October 20, 2018 08:17
test: stop using LAZY_LOAD_COUNT (istanbuljs#960)
feat: add support to exclude files on coverage report generation (istanbuljs#982)
With this change you can now tell nyc to clean the output directory when instrumenting, to do this an output directory must be specified.
I've currently set the default value for this to false so it can be a non-breaking change, although the default behaviour should be changed to true in the next major release.
I've included tests for this behaviour, and introduced lint support for `afterEach` and required the `makeDir` module
@coveralls
Copy link

coveralls commented Feb 28, 2019

Coverage Status

Coverage increased (+0.1%) to 93.834% when pulling 005f7f8 on AndrewFinlay:instrument-clean-option into 3b203c7 on istanbuljs:master.

@coreyfarrell
Copy link
Member

One concern I have with this PR is that the --clean flag has meaning in the global CLI context, it controls deletion of .nyc_output/. I'm not sure how I feel about having a sub command use the same flag name as the main command but for a different purpose.

CC @JaKXz @bcoe

… deleting cwd

The protection against deleting cwd is a little heavy handed, but I figure if you don't want to delete your cwd and probably don't want your output files going there either.
Added test support for these changes.
@AndrewFinlay AndrewFinlay changed the title feat: add clean option to instrument command feat: add delete option to instrument command Mar 7, 2019
@AndrewFinlay
Copy link
Contributor Author

After a chat with @coreyfarrell I've changed the name and added some level of defence against rimraf foot guns.

Copy link
Member

@bcoe bcoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this; I really like @coreyfarrell thinking that we should have checks and balances in place to protect us from overly eager rimrafs 😆

Copy link
Member

@coreyfarrell coreyfarrell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good once we have a chance to resolve the conflicts.

@coreyfarrell coreyfarrell merged commit d6db551 into istanbuljs:master Mar 12, 2019
@AndrewFinlay AndrewFinlay deleted the instrument-clean-option branch March 14, 2019 23:51
@coreyfarrell coreyfarrell mentioned this pull request Apr 3, 2019
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.

None yet

4 participants