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

Upgrade Moment to 2.19.x #98

Merged
merged 3 commits into from Dec 5, 2017

Conversation

@calmdev
Copy link
Contributor

calmdev commented Nov 30, 2017

Tell me if you want changes.

Lab reports a leak when I run tests locally.

17 tests complete
Test duration: 66 ms
Assertions count: 34 (verbosity: 2.00)
The following leaks were detected:WebAssembly
Coverage: 100.00%
Linting results: No issues

Resolves #97

Levi Lewis
@calmdev

This comment has been minimized.

Copy link
Contributor Author

calmdev commented Nov 30, 2017

Same deal on Travis. Not sure what the solution is here:

The following leaks were detected:SharedArrayBuffer, Atomics, WebAssembly
@calmdev

This comment has been minimized.

Copy link
Contributor Author

calmdev commented Nov 30, 2017

Seems related to Lab. These are whitelisted globals in newer versions.

Levi Lewis added 2 commits Dec 1, 2017
@calmdev

This comment has been minimized.

Copy link
Contributor Author

calmdev commented Dec 1, 2017

@arb can you review when you have a chance.

@@ -4,7 +4,6 @@


const Lab = require('lab');
const Code = require('code');

This comment has been minimized.

Copy link
@calmdev

calmdev Dec 1, 2017

Author Contributor

I don't know if this is the right solution, but using lab.expect instead of requiring code directly is what made the assert counts start working after upgrading to lab v14.

@justinffs

This comment has been minimized.

Copy link

justinffs commented Dec 4, 2017

The PR says you are upgrading moment but you are also upgrading lab

@Marsup
Marsup approved these changes Dec 4, 2017
@calmdev

This comment has been minimized.

Copy link
Contributor Author

calmdev commented Dec 5, 2017

@justinffs I wasn't intending to upgrade lab when the pull request was made, but without doing so the tests did not pass on travis CI as mentioned in the comments.

@arb arb self-assigned this Dec 5, 2017
@arb arb added this to the 6.4.1 milestone Dec 5, 2017
@arb arb merged commit f1b1a5b into hapijs:master Dec 5, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.