Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Feature - Request Image Content Types #33

Merged
merged 1 commit into from
Oct 14, 2015

Conversation

cnoon
Copy link
Member

@cnoon cnoon commented Oct 10, 2015

This PR adds the ability for a user to add additional acceptable image content types for validation. These changes address the challenges faced in both #28 and #29.

This change addresses the challenges faced in both #28 and #29.
@cnoon cnoon added this to the 2.0.0 milestone Oct 10, 2015
@cnoon cnoon self-assigned this Oct 10, 2015
@cnoon
Copy link
Member Author

cnoon commented Oct 10, 2015

cc @kcharwood and @jshier


- parameter contentTypes: The additional content types.
*/
public class func addAcceptableImageContentTypes(contentTypes: Set<String>) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to have a remove?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only scenario I think of for having a remove is a user who wants an invalid content type error for some reason. I can't think of a valid use, though more and more I'm coming to realize I have yet to experience the full range of terrible backend APIs. 👀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question @kcharwood. I think @jshier hit the only use case I can think of for requiring a removal, and I don't think it's a very realistic case. I could certainly add the API for completeness, but I think I'd prefer to leave it out now, and add it when we have a use case actually come up where it makes sense to add it.

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, just wanted to pose the question.

cnoon added a commit that referenced this pull request Oct 14, 2015
…ypes

Feature - Request Image Content Types
@cnoon cnoon merged commit 1ce7c6b into master Oct 14, 2015
@cnoon cnoon deleted the feature/request_image_content_types branch October 14, 2015 15:13
@cnoon cnoon mentioned this pull request Oct 14, 2015
@cnoon cnoon mentioned this pull request Sep 7, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants