Update dependencies #142
Update dependencies #142
Conversation
|
Dropping of NodeJS version 6 covered in #143 |
honzajavorek
force-pushed
the
honzajavorek/update-deps
branch
from
ac5c9e3
to
4b67e5c
Compare
honzajavorek
force-pushed
the
honzajavorek/update-deps
branch
from
4b67e5c
to
5270998
Compare
honzajavorek
force-pushed
the
honzajavorek/update-deps
branch
from
5270998
to
793bce4
Compare
|
@artem-zakharchenko Please look at the changes I made because of media-typer, whether they make sense to you. Thanks 🙏 |
| (parsed.type === 'application' && parsed.subtype === 'json') | ||
| || parsed.suffix === 'json' | ||
| ); | ||
| } catch (e) { |
There was a problem hiding this comment.
We shouldn't silence an error. Can we log it to console?
There was a problem hiding this comment.
The error gets thrown in case the value is not a valid media type. The only reponsibility of the function is to assess whether the value is JSON-related or not, so I think this is a valid approach.
The contentType value is basically a user input, it can be any kind of rubbish, and it is not Gavel's problem if it's invalid media type as far as it matches real/expected response.
There was a problem hiding this comment.
I added an explanatory comment in 519965b so it doesn't trigger a red flag when someone reads it
|
|
||
| return isJson; | ||
| try { | ||
| const { type } = contentTypeModule.parse(`${contentType}`); |
There was a problem hiding this comment.
The only thing that troubles me is too many things sharing what seems the same namespace.
Perhaps, contentType argument is not really a proper content type? Or a rawContentType..?
There was a problem hiding this comment.
Yeah, you're right basically I just hotfixed it. I'll try to make it nicer :)
There was a problem hiding this comment.
I renamed the variables in 519965b
|
🎉 This PR is included in version 3.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Parked until @artem-zakharchenko drops Node 6 here. Closes #125.
When updating
media-typer, I had to do something similar to this apiaryio/api-elements.js#205. I did not upgrade Cucumber as it proved to be a rabbit hole - documented under #116.