Exact Size #123
Exact Size #123
Conversation
| imagecolortransparent($dest_image, $background); | ||
| imagefill($dest_image, 0, 0, $background); | ||
| if( !empty($exact_size) && is_array($exact_size) ){ | ||
| $dest_image = imagecreate($exact_size[0], $exact_size[1]); |
There was a problem hiding this comment.
This seems to have the additional space.
| if( !empty($exact_size) && is_array($exact_size) ){ | ||
| $dest_image = imagecreate($exact_size[0], $exact_size[1]); | ||
| } else{ | ||
| $dest_image = imagecreate($this->getDestWidth(), $this->getDestHeight()); |
There was a problem hiding this comment.
This seems to have the additional space.
There was a problem hiding this comment.
What do you mean by "the additional space"?
There was a problem hiding this comment.
The white space seems to be existed before the $dest_image variable :).
lib/ImageResize.php
Outdated
| @@ -105,7 +105,7 @@ public function __construct($filename) | |||
| if (!defined('IMAGETYPE_WEBP')) { | |||
| define('IMAGETYPE_WEBP', 18); | |||
| } | |||
| if ($filename === null || empty($filename) || (substr($filename, 0, 5) !== 'data:' && !is_file($filename))) { | |||
| if ($filename === null || empty($filename) || (substr($filename, 0, 7) !== 'data://' && !is_file($filename))) { | |||
There was a problem hiding this comment.
The Travis CI build is broken because this image file checking codition is modified.
I think this should revert the original condition.
There was a problem hiding this comment.
This substr($filename, 0, 7) was on the master branch. The version I downloaded was the previous one. So I wasn't sure what to do.
There was a problem hiding this comment.
Well. Checking the latest merged PR seems that it's different.
There was a problem hiding this comment.
The master branch is ($filename, 0, 5). Please check that :-).
There was a problem hiding this comment.
Hi @felipetnh, thank you for your reply.
The Travis CI build work is broken and it should be fixed :).
Thank you for your PR.
Pull Request Test Coverage Report for Build 195
💛 - Coveralls |
1 similar comment
Pull Request Test Coverage Report for Build 195
💛 - Coveralls |
|
@felipetnh if you can fix the CI error, we can merge this. |
|
@adityapatadia, it seems that the CI work is successful, but the It looks like the code coverage is decreased and I think we need to add more tests to recover the code coverage... |
|
Guys, I have no idea what you're talking about.
CI errors? Coveralls?
Those things aren't really my thing.
Tell me what I need to do and I'll gladly update the PR.
Thanks!
Em qui, 4 de out de 2018 04:33, peter279k <[email protected]>
escreveu:
… @adityapatadia <https://github.com/adityapatadia>, it seems that the CI
work is successful, but the coveralls is failed.
It looks like the code coverage is decreased and I think we need to add
more tests to recover the code coverage...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#123 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEEkMyYyqnltoPncs7C2MAWKOzFsUbAfks5uhbnDgaJpZM4W3UaV>
.
|
|
Hi @felipetnh, do you see this captured image? The |
|
And how do I do that?
I've never worked with test cases.
Em qui, 4 de out de 2018 às 07:48, peter279k <[email protected]>
escreveu:
… Hi @felipetnh <https://github.com/felipetnh>, do you see this captured
image?
[image: image]
<https://user-images.githubusercontent.com/9021747/46469397-d17fb780-c805-11e8-89e8-cdcad0d2cd80.png>
The coverage/coveralls is failed because the coverage decreased. You need
to do is add more tests to recover the code coverage back 😄.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#123 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEEkM9rfKkk9RYyn-AAr9zMFHWER6xFsks5uhedkgaJpZM4W3UaV>
.
|
|
@felipetnh, thank you for your reply. Let me take a look at your forked repo and suggest the recommendations to increase code coverage :). |
|
After tracing the source code, it looks like that we don't have to increase code coverage because this decreased percent is small. @adityapatadia, I think it's time to be merged now. |
|
It would be great if we can also add details about this in Readme. |
|
Hi @adityapatadia, how about adding some details for this issue in released draft when releasing the latest version? |
|
Any update on this? |
|
Hi @ekinhbayar, thank you for your concern. Just wait for @adityapatadia review this PR. |
| imagegammacorrect($this->source_image, 2.2, 1.0); | ||
|
|
||
| if( !empty($exact_size) && is_array($exact_size) ) { |
There was a problem hiding this comment.
It's mentioned on issue #142.
There was a problem hiding this comment.
@adityapatadia, this codes will effect and cause on the issue #142.
The line 291 to 300.
@adityapatadia