Closed rjpcal closed 4 years ago
Merging #352 into master will not change coverage. The diff coverage is
100.00%
.
@@ Coverage Diff @@
## master #352 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 133 134 +1
=========================================
+ Hits 133 134 +1
Impacted Files | Coverage Ξ | |
---|---|---|
lib/index.js | 100.00% <100.00%> (ΓΈ) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Ξ = absolute <relative> (impact)
,ΓΈ = not affected
,? = missing data
Powered by Codecov. Last update 4e168c2...c09fdb2. Read the comment docs.
Hi @rjpcal this PR is good. (the code that you have added to index.js
makes sense and should fix issue #328) π
However I'm confused about the need for the ms
package.
I would have thought that if it were required for the pre-commit
hook, we would have been aware of the need ... π
Can you please try removing ms
from the package.json
and doing rm -rf node_modules
followed by npm install
and npm test
to confirm? π
Thanks for the quick review!
So strange, it did work fine now with ms
removed after your suggestion of removing+reinstalling node_modules
, and I've updated the branch now with ms
removed again. (What had failed the first time was the eslint
part of the pre-commit - error message pasted in an earlier comment - and I verified that's now working, just not sure why it failed at first.)
@rjpcal hapi-auth-jwt2@10.2.0
contains your update. π
Thanks again. π₯
@nelsonic that's great, thank you!
@rjpcal you did the work. I just had to merge+publish it. BTW: love it when CTOs still write code. π
π and I love it too!
This prevents a custom verify() function from being called with decoded=null, from which the function then has no way to avoid producing a 500 response to the request.