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: add missing description for --exclude-after-remap #413

Merged
merged 5 commits into from Jul 19, 2022
Merged

docs: add missing description for --exclude-after-remap #413

merged 5 commits into from Jul 19, 2022

Conversation

ArrayZoneYour
Copy link
Contributor

@ArrayZoneYour ArrayZoneYour commented Jul 17, 2022

Reference:

https://github.com/istanbuljs/nyc#source-map-support-for-pre-instrumented-codebases

#293

Checklist
  • documentation is changed or added

Copy link
Owner

@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 the contribution 👏

I left some suggestions about how we might better explain the difference between inline vs., on disk source maps, and the setting --exclude-after-remap.

README.md Outdated
transpiler like [`@babel/register`]) c8 supports both inline source-maps and
`.map` files.

_Important: If you are using c8 with a project that pre-instruments its code,
Copy link
Owner

Choose a reason for hiding this comment

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

It would be useful to give an example of when we would want to set --exclude-after-remap to true, vs, when we would want to leave it false, rather than this blurb.

README.md Outdated
@@ -53,6 +54,17 @@ By supplying `--all` to c8, all files in directories specified with `--src` (def
and `--exclude` flag checks, will be loaded into the report. If any of those files remain uncovered they will be factored
into the report with a default of 0% coverage.

## Source-Map support for pre-instrumented codebases
Copy link
Owner

Choose a reason for hiding this comment

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

Perhaps instead:

Pre-instrumented vs., just-in-time instrumented codebases

Source map files, vs., inline source maps

Just-in-time instrumented codebases will often insert source maps inline with the .js code they generate at runtime (_as an example, @babel-register/register can be configured to insert a source map foote).

Pre-instrumented codebases, e.g., running tsc to generate .js in a build folder, may generate either inline source maps, or a separate .map file stored on disk.

c8 can handle loading both types of source maps.

Exclude after remap

... explain here why you when you would set the setting to true, vs., false.

@ArrayZoneYour
Copy link
Contributor Author

@bcoe Thanks for suggestions and review 😀. Update the explanations and be willing to make some other changes if it is required.

README.md Outdated Show resolved Hide resolved
Copy link
Owner

@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.

Left a couple more editing suggestions, does this get at what you were picturing?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
ArrayZoneYour and others added 2 commits July 20, 2022 00:01
Co-authored-by: Benjamin E. Coe <bencoe@google.com>
Co-authored-by: Benjamin E. Coe <bencoe@google.com>
@ArrayZoneYour ArrayZoneYour requested a review from bcoe July 19, 2022 16:05
README.md Outdated Show resolved Hide resolved
@bcoe bcoe merged commit ac99234 into bcoe:main Jul 19, 2022
@bcoe
Copy link
Owner

bcoe commented Jul 19, 2022

@ArrayZoneYour thank you for the contribution.

mcknasty pushed a commit to mcknasty/c8 that referenced this pull request Feb 2, 2024
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

2 participants