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

docs: Update description of how files are selected for instrumentation #1009

Merged
merged 19 commits into from Mar 5, 2019

Conversation

AndrewFinlay
Copy link
Contributor

So I've put together a draft to better explain how files are included for coverage in nyc.

Take a look and tell me what you think, it probably needs a few changes here or there but I think it will take out a little of the mystery in configuring nyc.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.639% when pulling 497b871 on AndrewFinlay:include-exclude-doc into b64d921 on istanbuljs:master.

Copy link
Member

@JaKXz JaKXz left a comment

Choose a reason for hiding this comment

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

This is amazing, thank you so much @AndrewFinlay! Some minor nits and suggestions for you.

Also I think we can save some CI cycles if you just put [skip ci] in your commit messages :)

Last thing, what do you think of doing newlines at the end of sentences rather than the arbitrary line length? I think it makes reviewing changes a bit easier since each line is a sentence but I know there's this (IMO strange) pattern in place - maybe there's a good reason for it I don't know but I'm not a fan personally...

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@AndrewFinlay
Copy link
Contributor Author

@JaKXz thanks for the review,

I'll add the [skip ci] in future, simply didn't know it existed.

The line endings are simply an artefact of how I work with my editor. I'll fix it up tomorrow

@AndrewFinlay AndrewFinlay changed the title docs: Update description of how files are selected for coverage docs: Update description of how files are selected for instrumentation Mar 4, 2019
@JaKXz
Copy link
Member

JaKXz commented Mar 5, 2019

This is odd... the single newlines are being rendered as <br/>s... not sure why that's happening.

@coreyfarrell
Copy link
Member

This is odd... the single newlines are being rendered as
s... not sure why that's happening.

@JaKXz where are you seeing this?

@JaKXz
Copy link
Member

JaKXz commented Mar 5, 2019

@coreyfarrell in the rich diff

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
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.

Thanks for working on this @AndrewFinlay!

@JaKXz JaKXz merged commit a161d23 into istanbuljs:master Mar 5, 2019
@AndrewFinlay AndrewFinlay deleted the include-exclude-doc branch March 5, 2019 23:03
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