-
Notifications
You must be signed in to change notification settings - Fork 313
V.2 Candidate #163
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
V.2 Candidate #163
Conversation
@KarelWintersky
KarelWintersky
commented
Aug 7, 2020
- [+] More exceptions and error messages
- [+] Added ImageResizeInterface
- [*] Reorder and optimize code
- [*] GammaCorrection disabled by default (issue Continuous calls darken the image further and further. #162 )
- [*] Reformat code
- [*] Tests: updated
- [*] Fixes: composer.json
- [*] Fixes: README.md
* [+] More exceptions and error messages * [+] Added ImageResizeInterface * [*] Reorder and optimize code * [*] GammaCorrection disabled by default (issue #162) * [*] Reformat code * [*] Tests: updated * [*] Fixes: composer.json * [*] Fixes: README.md
* [*] fix source gamma correction at chaining save-calls
Two months...
bump?
Sorry. I fell of the stairs accidentally last Friday.
And I can't review your codes for now.
Once I recover, I will review your codes.
Well, I'll fork this and publish as our corporate-internal package.
If you can wait till weekend, I can review and merge it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider reverting the previous coding style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PHP compiler will optimize this code without any problems
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't understand you. You need in $resize local value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider following points:
- Reverting and be consistency for coding style.
- Checking the codes and remove useless new lines.
Please, give me linter configuration.
* [*] useless codestyle fixes
* [+] interface declarations
ties-s
commented
Dec 2, 2020
Any news on this? The docs claim chaining works but if I save multiple sizes this creates really dark images
@ties-s , maintainers of gumlet/php-image-resize rejected any changes or pull requests. You can use my package: ajur-media/php-image-resize
Sorry to my delay response.
Most of stuffs are good, but it looks like a breaking change.
It defines the Interface and some coding styles are not good.
If I've available time, I will try to fix some stuffs by myself.
ties-s
commented
Dec 9, 2020
It’s about the gamma correction. While this may be a breaking change, it’s also a bug fix. The documentation claims chaining works fine, which it doesn’t right now.
I think fixing this to align with docs and expected behavior should be a priority. You could always bump the version to indicate the breaking change.
It’s about the gamma correction. While this may be a breaking change, it’s also a bug fix. The documentation claims chaining works fine, which it doesn’t right now.
I think fixing this to align with docs and expected behavior should be a priority. You could always bump the version to indicate the breaking change.
I know that, but I need to review them carefully by myself.
Anyway, I will create another PR to work on that.
it’s also a bug fix.
this bug fixed in my package ;-)
@KarelWintersky this was closed by you and we did not reject this. It would be best if you can create smaller PRs which are easy to review and we can merge them quickly.
I really appreciate your contribution but moving too many things in one go can sometimes take more time to review. Hope you can help by creating smaller PRs.
I agree with @adityapatadia because this PR is too huge to review. And most of code structured are changed a lot.
Now I have no time for this, sorry.
Our project is under of emergency migration from MySQL to Postgresql. Deadline avg 26 Dec 2021.
No problem. The gamma issue is fixed in latest version. The but of chaining not working will be resolved sometime soon.
You delayed the process of reviewing my code for so long, and then made so many ridiculous and meaningless demands (by refusing to provide the linter configuration or a document about YOUR codestyle), that I lost all desire to help you fix errors in YOUR library.
You couldn't find time for more than two months to review changes that clearly improved the performance of your code and fixed many problems.
After that you made a lot of ridiculous claims about my work: "extra lines", "extra spaces", and so on and so forth.
Sorry, but I don't feel like investing in your library anymore. I corrected all the mistakes that you made during its development and released my own based on your code according to the license you specified.
You can use my code in your projects.