Skip to content

Conversation

@Ilshidur
Copy link
Contributor

@Ilshidur Ilshidur commented Dec 3, 2017

This PR refers to : #758

This PR features the following proposition : isMimeType(str).
It checks if the passed string matches to a correct MIME type format.

e.g. :

  • isMimeType('application/json') checks
  • isMimeType('application\json') DOES NOT check
  • isMimeType('application/json/text') DOES NOT check
  • isMimeType('') DOES NOT check
  • isMimeType('json') DOES NOT check

The reason this function does not match to exact MIME types listed by the IANA is because of lightness purposes : it would require to include all these MIME types in this library, which would weigh it significantly. This kind of effort maybe is not worth for the use that this function has in this entire library.

This function should check against all simple cases, but I'm not sure that it handles every exceptions and other "special" MIME type rules.

Waiting for someone's approval :-)

PS : This is the first PR of this kind that I submit. I'm really open to suggestions/corrections !

@profnandaa
Copy link
Member

Nicolas, I guess when you write DOES NOT check you mean, DOES NOT pass?

Copy link
Member

@profnandaa profnandaa 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 one thing, update the README.

@Ilshidur
Copy link
Contributor Author

Ilshidur commented Dec 5, 2017

@profnandaa Yes, I meant DOES NOT pass. Updated the README.

@Ilshidur Ilshidur changed the title [Waiting for approval] Feature : isMimeType(str) Feature : isMimeType(str) Dec 8, 2017
@chriso
Copy link
Collaborator

chriso commented Dec 9, 2017

Thanks for the PR 😄

@chriso chriso merged commit cd8fdfa into validatorjs:master Dec 9, 2017
@Ilshidur Ilshidur deleted the feature/isMimeType branch December 9, 2017 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants