Skip to content
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

Handle an invalid resource and return an empty GD resource #45

Merged
merged 2 commits into from
Jun 5, 2017

Conversation

GwendolenLynch
Copy link
Contributor

@GwendolenLynch GwendolenLynch commented Jun 1, 2017

Copy link
Member

@CarsonF CarsonF left a comment

Choose a reason for hiding this comment

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

I would rather throw an exception here, similar to createFromFile.
Side note: we could definitely update these two createFrom methods to accept an optional info. That would save having to create it again.

It's the logic in Creator::verifyInfo that needs to be updated. Instead of catching exception we need to check for false on isValid. We should tag FS as a feature release and then require that minor version here since we need that new method.

@GwendolenLynch
Copy link
Contributor Author

I would rather throw an exception here, similar to createFromFile.

OK, that is me misunderstanding this:

We should make sure thumbs handles the invalid object correctly instead of relying on an exception to be thrown.

… as you not wanting to throw them here for the same reason … what a relief! Throwing the exception 👍 😄

I should be back online early-ish in your day time there, we'll sort the FS stuff and tag, then revisit this from there.

@GwendolenLynch GwendolenLynch changed the base branch from 3.2 to 3.x June 4, 2017 15:47
@GwendolenLynch
Copy link
Contributor Author

OK, done and done.

@CarsonF CarsonF merged commit b036002 into 3.x Jun 5, 2017
@CarsonF CarsonF deleted the hotfix/invalid-images branch June 5, 2017 16:23
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.

None yet

2 participants