Add default resizer and default resizer adapter #721
Conversation
|
ping @rande |
|
ping @rande ) |
|
@rande i see other PRs have response, and why this is not ? Is it really not good for bundle ? |
|
Anybody ping?) |
|
ping @rande ) |
| if (extension_loaded('gmagick')) { | ||
| $this->isInstanceOf('Imagine\\Gmagick\\Imagine', $this->container->get('sonata.media.adapter.image.gmagick')); | ||
| } | ||
| if (!extension_loaded('gd') && !extension_loaded('imagick') && extension_loaded('gmagick')) { |
There was a problem hiding this comment.
Should be !extension_loaded('gmagick') I think.
|
I'm not an active user of this bundle, but I'll try to take a look soon. Can you please fix my quick review? |
|
@soullivaneuh yep, i'll do it but later this day |
|
How to pass "SonataCI - Quick Check" ? I missed smth from contributing rules? |
|
@hanovruslan I need to fix " SonataCi / Sandbox / PHPUnit — None ", there is nothing wrong on your side. |
Sounds good! |
|
Travis is still failing. |
|
Yep, i must learn that sonata has php 5.3 support. Sorry for that |
Fixed |
|
ping @rande can you please review this PR? thank you |
| } | ||
|
|
||
| /** | ||
| * @param \Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition $node |
There was a problem hiding this comment.
Don't use FQN. Import class instead.
|
@hanovruslan SonataCI is just a bot, you can't ping him ;) |
|
We're updating all version, please be patient |
|
@core23 Yep, i know that but maybe github has some feature like mail groups )) |
@core23 Should i just wait for something ? As you can see this PR does not play with deps. |
|
According to the new Sonata version management and next major release plan, this project has been refactored regarding branching and versioning. If you see this message, your PR concerns a patch or a minor release and is not targeting the right branch. So I'm closing this one, but don't see it as a refusal. If you think your work is still relevant and want to continue, feel free to reopen it on the right branch (e.g. the default one). Regards. |
|
is this a BC break? |
|
@OskarStark as far as i know - no, there is no BC break. But. I've done this PR long time ago (more than year) and i think test on real app would be helpful |
are you willing to to this? |
|
@OskarStark yep, but in about a week i guess. Not today for sure |
thanks for your effort 👍 |
| @@ -22,7 +22,7 @@ | |||
| class SimpleResizer implements ResizerInterface | |||
| { | |||
| /** | |||
| * @var ImagineInterface | |||
| * @var ImagineInterface. | |||
|
Please rebase and apply the new PR template @hanovruslan |
| $this->configureClassesToCompile(); | ||
| } | ||
|
|
||
| /** | ||
| * @param ContainerBuilder $container | ||
| * @param array $config | ||
| */ | ||
| public function configureProviders(ContainerBuilder $container, $config) | ||
| public function configureAdapters(ContainerBuilder $container, array $config) |
There was a problem hiding this comment.
Why public? Can we make this at least protected?
@hanovruslan in this case, please reopen this PR against the stable branch (3.x). Thanks again. 👍 Closing this one. |
Implementation of #700