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

fix: merge ranges properly when contained by other ranges in set #750

Merged
merged 1 commit into from
Nov 4, 2023

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Nov 2, 2023

@SimenB
Copy link
Member

SimenB commented Nov 3, 2023

@isaacs I'm not allowed to push to this branch - can you run npm run fix locally?

@@ -320,7 +411,7 @@ class FileCoverage {
ret.branches = this.computeBranchTotals('b');
// Tracking additional information about branch truthiness
// can be optionally enabled:
if (this['bt']) {
Copy link
Member

Choose a reason for hiding this comment

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

huh, typo before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems that way!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this was intentional, then the code should have a test, because it wasn't being covered.

Copy link
Member

Choose a reason for hiding this comment

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

digging through #639 and linked efforts, seems like this was simply a typo

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

this LGTM and only adds tests without changing old ones, so I think this is good to go 👍

Need to fix prettier for CI, tho

@isaacs
Copy link
Contributor Author

isaacs commented Nov 3, 2023

Fixed lint errors, PTAL, thanks :)

@SimenB SimenB merged commit 288888f into master Nov 4, 2023
5 checks passed
@github-actions github-actions bot mentioned this pull request Nov 4, 2023
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.

Branch merging broken when multiple files source-map to same origin file
2 participants