Refactor get_crop to always lookup the thumb from the database. #34
Conversation
| return | ||
|
|
||
| if size: | ||
| warnings.warn("The size kwarg is deprecated.", DeprecationWarning) | ||
|
|
||
| if exact_size: | ||
| warnings.warn("The exact_size kwarg is deprecated.", DeprecationWarning) | ||
|
|
There was a problem hiding this comment.
We now have two deprecated kwargs. Maybe we should write a new tag?
There was a problem hiding this comment.
That does the same thing as the old tag?
There was a problem hiding this comment.
I think that dropping two kwargs is a sign that we need to think about what went wrong with the design of this tag.
There was a problem hiding this comment.
For example, maybe a lot of the work the tag is doing should be in the model instead.
There was a problem hiding this comment.
We can remove the deprecation warnings if it makes you feel better. Passing size and exact size in have no effect besides to warn that they don't do anything.
There was a problem hiding this comment.
Unless you write some magic to pass the arguments through the relation, model logic won't work in the template.
There was a problem hiding this comment.
Well, it predates a lot of cropduster/generic-pluses functionality (prefetch etc). It makes sense than we'd refactor it to work with the updates.
The hard part about moving it onto the field is we'd have to get rid of crops with -'s in the names, or automatically infer they're the same as _, since we could do something like article.lead_image.lead_large in the template, but not article.lead_image.lead-large, iirc.
There was a problem hiding this comment.
I don't like article.lead_image.lead_large, but we could make it so the tag is just a simple call to a method with a cropname (which we wouldn't need in Jinja).
There was a problem hiding this comment.
That's what this is…
There was a problem hiding this comment.
It's still calling related_image, which is a model detail that breaks the Law of Demeter.
|
Before this is deployed we should grep for |
fdintino
force-pushed
the
get_crop-fixes
branch
from
25dc189
to
a3f54ce
Compare
|
@ctbarna similar question for this one. This is basically abandoned, right? |
No description provided.