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

Remove image cache #113

Merged
merged 2 commits into from
Aug 4, 2017
Merged

Remove image cache #113

merged 2 commits into from
Aug 4, 2017

Conversation

rodfersou
Copy link
Member

closes #109

@rodfersou
Copy link
Member Author

after this change, the image is working again: https://www.conversaafiada.com.br/brasil/melhor-discurso-do-lula-nao-vao-me-destruir

@rodfersou rodfersou force-pushed the issue_109 branch 2 times, most recently from 4a476ef to 886bb35 Compare July 18, 2017 11:49
CHANGES.rst Outdated
@@ -6,6 +6,9 @@ There's a frood who really knows where his towel is.
2.11 (unreleased)
^^^^^^^^^^^^^^^^^

- Remove image cache (closes `#109`_).
Copy link
Member

Choose a reason for hiding this comment

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

please write a proper changelog entry.

view, field = get_images_view(context)
if view:
view, field = get_images_view(context)
img = None
Copy link
Member

Choose a reason for hiding this comment

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

there's no need to initialize this variable here.

Copy link
Member Author

@rodfersou rodfersou Jul 19, 2017

Choose a reason for hiding this comment

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

yes, there is. In some case running in production the view or sizes object is None, and the method try to return a variable not initialized, resulting in an exception.

Copy link
Member

Choose a reason for hiding this comment

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

that variable is already set in lines 36, 52 and 54; there is no need to initialize it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hvelarde are you considering when view variable is None? I swear to you I did this because there was an exception without it, and this fixes the exception. you can check in the logs.

Also I don't want to move too much code in this PR, we have other issue for this.

@rodfersou rodfersou force-pushed the issue_109 branch 2 times, most recently from 30b6c8b to f753e78 Compare July 21, 2017 16:59
@hvelarde hvelarde merged commit 056874d into master Aug 4, 2017
@hvelarde hvelarde deleted the issue_109 branch August 4, 2017 20:01
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.

Facebook's Open Graph shows the logo of the site instead of the lead image
2 participants