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.

Sign up for GitHub

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

Jump to bottom

Some misc tweaks regarding eg. tap #77

Merged
merged 3 commits into from
Nov 14, 2021
Merged

Some misc tweaks regarding eg. tap #77

merged 3 commits into from
Nov 14, 2021

Conversation

voxpelli
Copy link
Contributor

I had trouble getting tap running locally on my node 16.11.1 + npm 8.1.1, not even the --legacy-bundling=true trick helped.

So, I did two things:

  • Updated to lkatest version of tap
  • Swapped the reporter to one which doesn't require react(!?)

And as an added fix:

  • I made it so that no package-lock.json gets generated locally

@voxpelli voxpelli mentioned this pull request Nov 14, 2021
Merged
jsumners
jsumners reviewed Nov 14, 2021
View reviewed changes
package.json Outdated Show resolved Hide resolved
@voxpelli voxpelli requested a review from jsumners November 14, 2021 16:23
mcollina
mcollina approved these changes Nov 14, 2021
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

Just bumping the dependency and cleaning up node_modules fixes it

@voxpelli
Copy link
Contributor Author

Looks like the coverage numbers suddenly doesn't match 100% any more 🤔

@voxpelli
Copy link
Contributor Author

Got confused by the code coverage flags and changes, the relevant change in tap@15 is that it now has --check-coverage on by default, which tap@14 didn't have.

Fixed by adding --no-check-coverage

jsumners
jsumners approved these changes Nov 14, 2021
View reviewed changes
Copy link
Member

@jsumners jsumners 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 to me.

@jsumners jsumners merged commit 3d4eb40 into pinojs:master Nov 14, 2021
@voxpelli voxpelli deleted the misc-improvements branch November 14, 2021 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@jsumners jsumners jsumners approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@voxpelli @mcollina @jsumners