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

ENH Add config to optionally disable image resampling #479

Merged
merged 2 commits into from Feb 20, 2022

Conversation

leochenftw
Copy link
Contributor

@michalkleiner
Copy link
Contributor

Thank you for raising the PR, @leochenftw. I saw the linked PR as well and I think it makes sense to allow for this configuration to be added, even on the File class despite there being Image class as well (which doesn't do much on its own).
I've suggested to reverse the name into a positive form (to reflect what the default is) and that the intention is to allow to optionally turn the resampling off.
If you could force-push an update and also change the commit message to read "ENH Add config to optionally disable image resampling" I'd be happy to merge that.

Thanks!

src/File.php Outdated Show resolved Hide resolved
Co-authored-by: Michal Kleiner <mk@011.nz>
@leochenftw leochenftw changed the title Set default value for no_resample ENH Add config to optionally disable image resampling Feb 20, 2022
@michalkleiner
Copy link
Contributor

Thanks for the tweaks @leochenftw. Since this is an enhancement, the PR needs to be based off and target the 1 branch.
Can you please also squash the commits so that it's just a single commit with the message I suggested? I can squash the commits at our end, but I can't rebase the PR.

@leochenftw leochenftw changed the base branch from 1.10 to 1 February 20, 2022 19:09
@leochenftw
Copy link
Contributor Author

thanks, @michalkleiner , I've rebased it to 1

@michalkleiner michalkleiner merged commit 8ed643d into silverstripe:1 Feb 20, 2022
@leochenftw leochenftw deleted the 1.10 branch February 20, 2022 21:31
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