Skip to content

Remove more Symfony 4 specific code #443

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 24, 2023
Merged

Remove more Symfony 4 specific code #443

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e813cc
Remove more Symfony 4 specific code
glaubinix Nov 21, 2023
f350eb2
Remove the services_legacy.xml to which previously set up an alias fo…
glaubinix Nov 23, 2023
a1cad05
Update CHANGELOG.md
dbu Nov 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions src/DependencyInjection/HttplugExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use Symfony\Component\HttpKernel\Kernel;
use Twig\Environment as TwigEnvironment;

/**
Expand All @@ -60,11 +59,7 @@ public function load(array $configs, ContainerBuilder $container): void

$loader->load('services.xml');
// TODO: Move this back into services.xml when we drop support for Symfony 4, or completely remove the service in the next major version.
if (Kernel::MAJOR_VERSION >= 5) {
$loader->load('services_legacy.xml');
} else {
$loader->load('services_legacy_sf4.xml');
}
$loader->load('services_legacy.xml');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could completely remove the legacy services and mention in the changelog what the new names to use are. or prefer to do that in a separate MR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can do this here. I take it, this means to also remove the service alias in the services.xml?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Psr\Http\Client\ClientInterface should still be autowireable. so i think that should stay.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I misunderstood what this when I first adjusted the xml config. Both legacy XML files triggered a deprecation if Http\Client\HttpClient was used. I first read this the wrong way and thought it created a httplug.client alias.

$loader->load('plugins.xml');
if (\class_exists(MockClient::class)) {
$loader->load('mock-client.xml');
Expand Down
5 changes: 1 addition & 4 deletions tests/Functional/ServiceInstantiationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\HttpFoundation\Request as SymfonyRequest;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\KernelInterface;
use Symfony\Component\HttpKernel\Profiler\Profiler;
Expand Down Expand Up @@ -132,8 +130,7 @@ protected static function bootKernel(array $options = []): KernelInterface
/** @var EventDispatcherInterface $dispatcher */
$dispatcher = static::$kernel->getContainer()->get('event_dispatcher');

$class = (Kernel::MAJOR_VERSION >= 5) ? RequestEvent::class : GetResponseEvent::class;
$event = new $class(static::$kernel, SymfonyRequest::create('/'), HttpKernelInterface::MASTER_REQUEST);
$event = new RequestEvent(static::$kernel, SymfonyRequest::create('/'), HttpKernelInterface::MAIN_REQUEST);

$dispatcher->dispatch($event, KernelEvents::REQUEST);

Expand Down
5 changes: 1 addition & 4 deletions tests/Unit/Collector/PluginClientFactoryListenerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
use Http\HttplugBundle\Collector\PluginClientFactoryListener;
use Nyholm\NSA;
use PHPUnit\Framework\TestCase;
use Symfony\Component\EventDispatcher\Event as LegacyEvent;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Component\Stopwatch\Stopwatch;
use Symfony\Contracts\EventDispatcher\Event;

Expand All @@ -28,8 +26,7 @@ public function testRegisterPluginClientFactory(): void

$listener = new PluginClientFactoryListener($factory);

$class = (Kernel::MAJOR_VERSION >= 5) ? Event::class : LegacyEvent::class;
$listener->onEvent(new $class());
$listener->onEvent(new Event());

$this->assertTrue(is_callable(NSA::getProperty(DefaultPluginClientFactory::class, 'factory')));
}
Expand Down