-
Notifications
You must be signed in to change notification settings - Fork 19
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
PP-225: Hapi v17, Node.js v14, update deps, nyc coverage, modernize #36
Conversation
tlhunter
commented
Aug 31, 2021
•
edited
Loading
edited
- update packages
- istanbul to nyc
- test on Node.js v14
- Hapi v17 compatibility
- updated linter rules
- SemVer MAJOR change
.gitignore
Outdated
@@ -11,7 +11,7 @@ pids | |||
lib-cov | |||
|
|||
# Coverage directory used by tools like istanbul |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit: if we're deprecating istanbul, we can say nyc
instead
.node-version
Outdated
14.15.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i believe some of our main repos are in 14.17.0
now, so we can bump this up a bit more
exports.register.attributes = { | ||
name: 'hapi-rate-limiter' | ||
}; | ||
exports.pkg = require('../package.json'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can also specify exports.name = 'hapi-rate-limiter'
if you want parity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the .pkg.name
and .pkg.version
get hoisted up to .name
and .version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh that's awesome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
few nits but LGTM!