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

Adding a Free Transform option #552

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open

Conversation

kckaiwei
Copy link
Contributor

For those times when you do not want/need scaling with image ratio (for one reason or another). This option will forcibly rescale/stretch the image to the supplied dimensions.

@coveralls
Copy link

coveralls commented Jul 29, 2018

Coverage Status

Coverage remained the same at ?% when pulling 1fda7a4 on kckaiwei:transform into b8e92e7 on jazzband:master.

@kckaiwei
Copy link
Contributor Author

Could use a 2nd eye on this. Adding an option changed the key used for caching (since the key is partially made by options supplied), so I updated the paths for the tests.

@matthiask
Copy link
Member

Can't the change of keys be avoided somehow? A change would be quite bad for big sites with lots of images.

@kckaiwei
Copy link
Contributor Author

That's a good point. I guess it could go in with the EXTRA_OPTIONS, and thus avoiding the need to recache every image. But the current EXTRA_OPTIONS all felt to be reflected as global variables set in the settings file. Although the docs do have a blurb on setting orientation on a image by image basis so there seems to be a precedent on it.

I'll make the changes accordingly.

@kckaiwei kckaiwei added WIP and removed needs-review labels Sep 13, 2018
@kckaiwei kckaiwei added needs-review and removed WIP labels Sep 14, 2018
@RDIL
Copy link
Contributor

RDIL commented Jan 11, 2019

Conflicts need fixing

@kckaiwei kckaiwei added WIP and removed needs-review labels Jun 1, 2020
@kckaiwei
Copy link
Contributor Author

kckaiwei commented Jun 1, 2020

Ensuring resolved conflicts still work, set to WIP

@camilonova
Copy link
Member

@kckaiwei are we ready to merge this?

@kckaiwei
Copy link
Contributor Author

No, I actually forgot about this tbh, I don't work at the same place that used it last time, so lemme see if I can get a test running for this, iirc. I had a bunch of failing tests on Travis last time due to it that I couldn't figure out.

@camilonova
Copy link
Member

Thanks. Take a look and let us know. If you think there is no way to get this up and running we better close this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants