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

added a micro kernel #15990

Merged
merged 1 commit into from Nov 5, 2015

Conversation

Projects
None yet
@fabpot
Member

fabpot commented Sep 29, 2015

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Related to #15948 and #15820

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Sep 29, 2015

Member

Obviously, there are a few minor todos as you mentioned, but this accomplishes all of the goals of my other PR's that you listed, but indeed, in a much simpler way (and a smaller code footprint). I'm very pleased by this version!

Member

weaverryan commented Sep 29, 2015

Obviously, there are a few minor todos as you mentioned, but this accomplishes all of the goals of my other PR's that you listed, but indeed, in a much simpler way (and a smaller code footprint). I'm very pleased by this version!

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Oct 1, 2015

Member

The updated routing method looks good to me. 👍 in general - except for the name of course :)

Member

weaverryan commented Oct 1, 2015

The updated routing method looks good to me. 👍 in general - except for the name of course :)

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Oct 1, 2015

Member

@fabpot if we want some tests (there's not much to test, but there could be some) or other changes and I can help - let me know and I can steal this PR and make those changes (and take all the credit!)

Member

weaverryan commented Oct 1, 2015

@fabpot if we want some tests (there's not much to test, but there could be some) or other changes and I can help - let me know and I can steal this PR and make those changes (and take all the credit!)

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Oct 1, 2015

Member

Here is a small usage example:

<?php

require_once __DIR__.'/../vendor/autoload.php';

use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\Debug\Debug;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\MicroKernel;
use Symfony\Component\Routing\RouteCollectionBuilder;

class AppKernel extends MicroKernel
{
    public function home()
    {
        return new Response('Foo');
    }

    public function registerBundles()
    {
        return array(
            new Symfony\Bundle\FrameworkBundle\FrameworkBundle(),
        );
    }

    protected function configureRoutes(RouteCollectionBuilder $routes)
    {
        $routes->add('/', 'kernel:home');
    }

    protected function configureServices(ContainerBuilder $c, LoaderInterface $loader)
    {
        $c->setParameter('kernel.secret', 'foo');
    }
}

$kernel = new AppKernel('dev', true);
$request = Request::createFromGlobals();
$response = $kernel->handle($request);
$response->send();
$kernel->terminate($request, $response);

I'm not quite satisfied as we cannot use a closure for controllers (as Symfony dumps the routes as a PHP file).

Member

fabpot commented Oct 1, 2015

Here is a small usage example:

<?php

require_once __DIR__.'/../vendor/autoload.php';

use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\Debug\Debug;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\MicroKernel;
use Symfony\Component\Routing\RouteCollectionBuilder;

class AppKernel extends MicroKernel
{
    public function home()
    {
        return new Response('Foo');
    }

    public function registerBundles()
    {
        return array(
            new Symfony\Bundle\FrameworkBundle\FrameworkBundle(),
        );
    }

    protected function configureRoutes(RouteCollectionBuilder $routes)
    {
        $routes->add('/', 'kernel:home');
    }

    protected function configureServices(ContainerBuilder $c, LoaderInterface $loader)
    {
        $c->setParameter('kernel.secret', 'foo');
    }
}

$kernel = new AppKernel('dev', true);
$request = Request::createFromGlobals();
$response = $kernel->handle($request);
$response->send();
$kernel->terminate($request, $response);

I'm not quite satisfied as we cannot use a closure for controllers (as Symfony dumps the routes as a PHP file).

@fabpot fabpot changed the title from [WIP] added a micro kernel to added a micro kernel Oct 1, 2015

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Oct 1, 2015

Member

Question: is it really something we want to package in Symfony itself. As you can see, the code is quite minimal, so isn't it something for a small/micro distribution? Or part of a tutorial in the docs? For me, that's more like a starting point rather than some kind of infrastructure that needs to be standardized.

Member

fabpot commented Oct 1, 2015

Question: is it really something we want to package in Symfony itself. As you can see, the code is quite minimal, so isn't it something for a small/micro distribution? Or part of a tutorial in the docs? For me, that's more like a starting point rather than some kind of infrastructure that needs to be standardized.

@xabbuh

This comment has been minimized.

Show comment
Hide comment
@xabbuh

xabbuh Oct 1, 2015

Member

If this is something that will be asked for frequently, I would say yes (personally I don't see any use cases for myself right now). The code that would reside inside the core looks good so far.

Member

xabbuh commented Oct 1, 2015

If this is something that will be asked for frequently, I would say yes (personally I don't see any use cases for myself right now). The code that would reside inside the core looks good so far.

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Oct 1, 2015

Member

The fact that closures cannot be used defeats the initial purpose (at least for me). With closure support, that would make it something interesting for small websites, APIs, things that Silex is good at.

Member

fabpot commented Oct 1, 2015

The fact that closures cannot be used defeats the initial purpose (at least for me). With closure support, that would make it something interesting for small websites, APIs, things that Silex is good at.

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Oct 1, 2015

Member

@fabpot I want the code somewhere where people can just use it, so people can get a minimal Symfony project up and running really quickly and with minimal setup. If we make the user do that boilerplate, it defeats the purpose.

My vote is for core because I want say: "you can create a symfony app by requiring symfony/symfony and creating a single file that extends MicroKernel" (or whatever we call it).

Also, I think the kernel should be used in the SE: I think loads things in a way that's more clear, but basically works the same (you could easily remove routing_dev.yml for example).

And finally, yes, Silex is awesome. But it doesn't have bundle support, which means I forfeit a lot of solutions I might be accustomed to using. A Silex app also can't evolve into a full Symfony app very easily if it grows. The idea behind this was - as much as possible - to have my cake and eat it too: get full Symfony, but get it small.

Anyways, I see your point - after all, this class is quite small - but that's my vote. We could move it into FrameworkBundle if we want to hide it a bit from the core components (especially since it requires FrameworkBundle.

Member

weaverryan commented Oct 1, 2015

@fabpot I want the code somewhere where people can just use it, so people can get a minimal Symfony project up and running really quickly and with minimal setup. If we make the user do that boilerplate, it defeats the purpose.

My vote is for core because I want say: "you can create a symfony app by requiring symfony/symfony and creating a single file that extends MicroKernel" (or whatever we call it).

Also, I think the kernel should be used in the SE: I think loads things in a way that's more clear, but basically works the same (you could easily remove routing_dev.yml for example).

And finally, yes, Silex is awesome. But it doesn't have bundle support, which means I forfeit a lot of solutions I might be accustomed to using. A Silex app also can't evolve into a full Symfony app very easily if it grows. The idea behind this was - as much as possible - to have my cake and eat it too: get full Symfony, but get it small.

Anyways, I see your point - after all, this class is quite small - but that's my vote. We could move it into FrameworkBundle if we want to hide it a bit from the core components (especially since it requires FrameworkBundle.

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Oct 2, 2015

Member

Also, I don't envision that you'll normally have just one file, though that ability is cool. More that it's easy to create a minimal application easily: a kernel, controller classes, services and some configuration files if you choose.

Member

weaverryan commented Oct 2, 2015

Also, I don't envision that you'll normally have just one file, though that ability is cool. More that it's easy to create a minimal application easily: a kernel, controller classes, services and some configuration files if you choose.

@wouterj

This comment has been minimized.

Show comment
Hide comment
@wouterj

wouterj Oct 2, 2015

Member

I don't like the idea of using this in the Standard Edition. The Standard Edition should be the base template for common Symfony applications, which tend to be quite advanced. This still is a nice feature, but for 20% or so of the Symfony developers.

What about creating a new package, like symfony/micro, which requires symfony/symfony and this class. Then you can say "Require the symfony/micro package, use the MicroKernel class and you're ready".

Member

wouterj commented Oct 2, 2015

I don't like the idea of using this in the Standard Edition. The Standard Edition should be the base template for common Symfony applications, which tend to be quite advanced. This still is a nice feature, but for 20% or so of the Symfony developers.

What about creating a new package, like symfony/micro, which requires symfony/symfony and this class. Then you can say "Require the symfony/micro package, use the MicroKernel class and you're ready".

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Oct 2, 2015

Member

What about inside FrameworkBundle? Having a new package for this one class looks like overkill, and this is just one small class.

Member

weaverryan commented Oct 2, 2015

What about inside FrameworkBundle? Having a new package for this one class looks like overkill, and this is just one small class.

@ogizanagi

This comment has been minimized.

Show comment
Hide comment
@ogizanagi

ogizanagi Oct 2, 2015

Member

👍 for FrameworkBundle

Member

ogizanagi commented Oct 2, 2015

👍 for FrameworkBundle

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Oct 5, 2015

Member

Is it too late to have this class considered to be merged?

Member

weaverryan commented Oct 5, 2015

Is it too late to have this class considered to be merged?

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Oct 5, 2015

Member

Honestly, I'm not very sure we need to include this class in Symfony. As you can see, the amount of code is minimal and advocating several ways of doing the same thing is always bad. Also, I don't see much benefits in using this instead of what we already have.

So, before merging this, I would want to understand the real use cases of this and why it's better than the current way. We can even decide to make it the preferred/only way at some point.

To sum up, as the code is minimal and because we can easily write a cookbook, I don't want to merge it now for 2.8/3.0.

Member

fabpot commented Oct 5, 2015

Honestly, I'm not very sure we need to include this class in Symfony. As you can see, the amount of code is minimal and advocating several ways of doing the same thing is always bad. Also, I don't see much benefits in using this instead of what we already have.

So, before merging this, I would want to understand the real use cases of this and why it's better than the current way. We can even decide to make it the preferred/only way at some point.

To sum up, as the code is minimal and because we can easily write a cookbook, I don't want to merge it now for 2.8/3.0.

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Oct 5, 2015

Member

@fabpot Ok, let me state my case! :).

Yes: there should only be one way to do things, and I've already removed a lot from the "micro kernel" idea for this. That's why I think this should be used in the SE.

1) Why would this be the preferred way?

Here's the diff to make this the kernel for the SE: https://gist.github.com/weaverryan/25351e929a5a0d04c1e6/revisions#diff-237b910f44f219a85dbcda0e21295717. (it could be even smaller - if I didn't remove routing_dev.yml)

So why is this a better way?

A) Container loading is the same as now + you have access to the ContainerBuilder.

B) Route loading, same as now, but your code is in PHP, so you can use some logic, e.g. loading
extra routes in dev/test environments, or building routes in PHP.

This could be enough to accept this class into core.

2) Use-cases

A) Creating a micro-service or a demo with minimal files

In this kernel, the loading of container/routing resources is done very explicitly. So it would be very natural to start with the SE (or start from scratch) and create an app with less configuration files / directories. But, it would also still look very similar to a traditional SE project: everything runs through configure***() methods in both cases, which are explicit and clear in their purpose.

The setup for the service route loader and closure container loader are boilerplate, and making the
user go through this makes doing this less attractive, especially for beginners:, since the setup is quite advanced. But, if it's available as a core class, looking at the source code later could be used for teaching.

B) Clarity for learning.

I first teach that Symfony is a routing/controller/response system. What simpler way than to show them the configureRoutes() method with a simple PHP route first that points to a controller? Then, we could naturally refactor to loading configuration files. The same goes for the container: I explain that we "teach" the container how to instantiate our services. Doing that in a method called configureServices() using native PHP at first would be very natural. Then we can load a YAML file (from that same method) as a different way of organization.

Again, if you require the route/container loader setup, that'll be a problem. Even if I did this before the class for them, the code is still sitting there, exposing complexity I have to tell them to ignore. And, their project wouldn't look like the SE - eventually we'd have to refactor this back to the "normal" method.

Thanks!

Member

weaverryan commented Oct 5, 2015

@fabpot Ok, let me state my case! :).

Yes: there should only be one way to do things, and I've already removed a lot from the "micro kernel" idea for this. That's why I think this should be used in the SE.

1) Why would this be the preferred way?

Here's the diff to make this the kernel for the SE: https://gist.github.com/weaverryan/25351e929a5a0d04c1e6/revisions#diff-237b910f44f219a85dbcda0e21295717. (it could be even smaller - if I didn't remove routing_dev.yml)

So why is this a better way?

A) Container loading is the same as now + you have access to the ContainerBuilder.

B) Route loading, same as now, but your code is in PHP, so you can use some logic, e.g. loading
extra routes in dev/test environments, or building routes in PHP.

This could be enough to accept this class into core.

2) Use-cases

A) Creating a micro-service or a demo with minimal files

In this kernel, the loading of container/routing resources is done very explicitly. So it would be very natural to start with the SE (or start from scratch) and create an app with less configuration files / directories. But, it would also still look very similar to a traditional SE project: everything runs through configure***() methods in both cases, which are explicit and clear in their purpose.

The setup for the service route loader and closure container loader are boilerplate, and making the
user go through this makes doing this less attractive, especially for beginners:, since the setup is quite advanced. But, if it's available as a core class, looking at the source code later could be used for teaching.

B) Clarity for learning.

I first teach that Symfony is a routing/controller/response system. What simpler way than to show them the configureRoutes() method with a simple PHP route first that points to a controller? Then, we could naturally refactor to loading configuration files. The same goes for the container: I explain that we "teach" the container how to instantiate our services. Doing that in a method called configureServices() using native PHP at first would be very natural. Then we can load a YAML file (from that same method) as a different way of organization.

Again, if you require the route/container loader setup, that'll be a problem. Even if I did this before the class for them, the code is still sitting there, exposing complexity I have to tell them to ignore. And, their project wouldn't look like the SE - eventually we'd have to refactor this back to the "normal" method.

Thanks!

@javiereguiluz

This comment has been minimized.

Show comment
Hide comment
@javiereguiluz

javiereguiluz Oct 5, 2015

Member

Another use case: create a microkernel to use it in functional tests of Symfony bundles.

Member

javiereguiluz commented Oct 5, 2015

Another use case: create a microkernel to use it in functional tests of Symfony bundles.

@jakzal

This comment has been minimized.

Show comment
Hide comment
@jakzal

jakzal Oct 15, 2015

Member

What about inside FrameworkBundle? Having a new package for this one class looks like overkill, and this is just one small class.

I wanted to write that this class could be used without the FrameworkBundle, but then I noticed it uses the framework extension. In which case the FrameworkBundle is probably the right place for it (unless we can make that extension is loaded optionally). Either way, I'd love to see this in core. I can see myself using it for simple projects, prototypes, workshop applications, or as Javier mentioned functional tests.

How about symfony/micro-edition?

Member

jakzal commented Oct 15, 2015

What about inside FrameworkBundle? Having a new package for this one class looks like overkill, and this is just one small class.

I wanted to write that this class could be used without the FrameworkBundle, but then I noticed it uses the framework extension. In which case the FrameworkBundle is probably the right place for it (unless we can make that extension is loaded optionally). Either way, I'd love to see this in core. I can see myself using it for simple projects, prototypes, workshop applications, or as Javier mentioned functional tests.

How about symfony/micro-edition?

@Pierstoval

This comment has been minimized.

Show comment
Hide comment
@Pierstoval

Pierstoval Oct 29, 2015

Contributor

I totally agree with @weaverryan , this code should be in the core. It'd be an awesome step for a "Symfony nude/empty/micro/small/light Edition" that would require only the FrameworkBundle and a single Controller class.

Contributor

Pierstoval commented Oct 29, 2015

I totally agree with @weaverryan , this code should be in the core. It'd be an awesome step for a "Symfony nude/empty/micro/small/light Edition" that would require only the FrameworkBundle and a single Controller class.

@henrikbjorn

This comment has been minimized.

Show comment
Hide comment
@henrikbjorn

henrikbjorn Oct 29, 2015

Contributor

👍 i like this idea as it does a lot of the stuff that i am trying to tie into Silex eg. the Routing.

Contributor

henrikbjorn commented Oct 29, 2015

👍 i like this idea as it does a lot of the stuff that i am trying to tie into Silex eg. the Routing.

@Pierstoval

This comment has been minimized.

Show comment
Hide comment
@Pierstoval

Pierstoval Oct 29, 2015

Contributor

I reviewed the code a bit, and actually, as this kernel is abstract, I think it could be good for the different configure* methods to be abstract too.
There are three methods to be overriden, for routes, services and extensions.
With the registerBundles function, it means that there will be at least 4 methods defined in application kernel. If this abstract kernel is used, I bet it will be unlikely that the 3 abstracted methods would be empty. And if they're empty, it's not a problem: there are many small apps where the services.yml file is empty, or routing.yml contains only the AppBundle/Controller/ annotation loader for every controller.

It'd allow a better start to understand how a kernel can be configured this way IMO.

Contributor

Pierstoval commented Oct 29, 2015

I reviewed the code a bit, and actually, as this kernel is abstract, I think it could be good for the different configure* methods to be abstract too.
There are three methods to be overriden, for routes, services and extensions.
With the registerBundles function, it means that there will be at least 4 methods defined in application kernel. If this abstract kernel is used, I bet it will be unlikely that the 3 abstracted methods would be empty. And if they're empty, it's not a problem: there are many small apps where the services.yml file is empty, or routing.yml contains only the AppBundle/Controller/ annotation loader for every controller.

It'd allow a better start to understand how a kernel can be configured this way IMO.

@mfdj

This comment has been minimized.

Show comment
Hide comment
@mfdj

mfdj Oct 29, 2015

I like how flat this Kernel is (in the PEP20 sense)… FlatKernel is probably obtuse but it resonates with the design characteristic.

A great reason to have this in the core is it enables demonstrating the framework to a non-Symfony (or even non-PHP) developer. An app powered by a single file — with no disclaimers about non-standard approach! — very appealing to the uninitiated.

mfdj commented Oct 29, 2015

I like how flat this Kernel is (in the PEP20 sense)… FlatKernel is probably obtuse but it resonates with the design characteristic.

A great reason to have this in the core is it enables demonstrating the framework to a non-Symfony (or even non-PHP) developer. An app powered by a single file — with no disclaimers about non-standard approach! — very appealing to the uninitiated.

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Oct 30, 2015

Member

I've just made some changes:

  • moved the class to FrameworkBundle;
  • merged the configureServices() and configureExtensions() into one new configureContainer() method;
  • made the two configure*() method abstract.

"We" want to merge that into 2.8, so ping @symfony/deciders

Member

fabpot commented Oct 30, 2015

I've just made some changes:

  • moved the class to FrameworkBundle;
  • merged the configureServices() and configureExtensions() into one new configureContainer() method;
  • made the two configure*() method abstract.

"We" want to merge that into 2.8, so ping @symfony/deciders

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Oct 30, 2015

Member

👍

Member

fabpot commented Oct 30, 2015

👍

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Oct 30, 2015

Member

And now with some tests \o/

Member

fabpot commented Oct 30, 2015

And now with some tests \o/

@dunglas

This comment has been minimized.

Show comment
Hide comment
@dunglas

dunglas Oct 30, 2015

Member

👍

Member

dunglas commented Oct 30, 2015

👍

@Pierstoval

This comment has been minimized.

Show comment
Hide comment
@Pierstoval

Pierstoval Oct 30, 2015

Contributor

Yeah, trick or treat!

Can't wait for this kernel to be alive in 2.8, it'd be a great move, and I bet there will be many "Symfony micro-apps" in the future :)

Contributor

Pierstoval commented Oct 30, 2015

Yeah, trick or treat!

Can't wait for this kernel to be alive in 2.8, it'd be a great move, and I bet there will be many "Symfony micro-apps" in the future :)

@aitboudad

This comment has been minimized.

Show comment
Hide comment
@aitboudad

aitboudad Oct 31, 2015

Contributor

👍

Contributor

aitboudad commented Oct 31, 2015

👍

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Nov 4, 2015

Member

Making this class compatible with PHP 5.3 is a PITA. What about making it require PHP 5.5?

Member

fabpot commented Nov 4, 2015

Making this class compatible with PHP 5.3 is a PITA. What about making it require PHP 5.5?

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Nov 4, 2015

Member

@fabpot is there something beyond the ugly $that? I don't think requiring 5.5 will cause many practical issues, but the $that doesn't seem like a deal breaker (and we could change it in 3.0 immediately). But if there is some other issue, then 5.5 is ok - I just want it to be in :)

Member

weaverryan commented Nov 4, 2015

@fabpot is there something beyond the ugly $that? I don't think requiring 5.5 will cause many practical issues, but the $that doesn't seem like a deal breaker (and we could change it in 3.0 immediately). But if there is some other issue, then 5.5 is ok - I just want it to be in :)

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Nov 5, 2015

Member

👍

Member

weaverryan commented Nov 5, 2015

👍

@dunglas

This comment has been minimized.

Show comment
Hide comment
@dunglas

dunglas Nov 5, 2015

Member

👍 the trait looks good.

Member

dunglas commented Nov 5, 2015

👍 the trait looks good.

@fabpot fabpot merged commit eab0f0a into symfony:2.8 Nov 5, 2015

3 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Nov 5, 2015

feature #15990 added a micro kernel (fabpot)
This PR was merged into the 2.8 branch.

Discussion
----------

added a micro kernel

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Related to #15948 and #15820

Commits
-------

eab0f0a added a micro kernel
@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Nov 5, 2015

Member

Thanks Fabien :)

Member

weaverryan commented Nov 5, 2015

Thanks Fabien :)

@Pierstoval

This comment has been minimized.

Show comment
Hide comment
@Pierstoval

Pierstoval Nov 5, 2015

Contributor

👍 Can't wait to see where it'll get Symfony !

Contributor

Pierstoval commented Nov 5, 2015

👍 Can't wait to see where it'll get Symfony !

@henrikbjorn

This comment has been minimized.

Show comment
Hide comment
@henrikbjorn

henrikbjorn Nov 5, 2015

Contributor

While playing around with it, i found it was hard to get to import another routing file using RouteCollectionBuilder, because its import() function return a new builder.

Is there a better way of accomplishing this (notice the & which replaces the builder):

    protected function configureRoutes(RouteCollectionBuilder &$routes)
    {
        $routes = $routes->import($this->rootDir . '/config/routing.yml', null);
    }
Contributor

henrikbjorn commented Nov 5, 2015

While playing around with it, i found it was hard to get to import another routing file using RouteCollectionBuilder, because its import() function return a new builder.

Is there a better way of accomplishing this (notice the & which replaces the builder):

    protected function configureRoutes(RouteCollectionBuilder &$routes)
    {
        $routes = $routes->import($this->rootDir . '/config/routing.yml', null);
    }
@GromNaN

This comment has been minimized.

Show comment
Hide comment
@GromNaN

GromNaN Nov 5, 2015

Contributor

The method configureRoutes could have to return the $routes.

Contributor

GromNaN commented Nov 5, 2015

The method configureRoutes could have to return the $routes.

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Nov 5, 2015

Member

@henrikbjorn thanks for playing with it. Here is how you're supposed to import routes, but there could be a usability problem (as you are quite smart and this wasn't obvious to you):

$routes->mount('/', $routes->import(__DIR__.'/config/routing.yml');

In other words, it does create a new RouteCollectionBuilder, but you still need to mount it into the existing builder.

Member

weaverryan commented Nov 5, 2015

@henrikbjorn thanks for playing with it. Here is how you're supposed to import routes, but there could be a usability problem (as you are quite smart and this wasn't obvious to you):

$routes->mount('/', $routes->import(__DIR__.'/config/routing.yml');

In other words, it does create a new RouteCollectionBuilder, but you still need to mount it into the existing builder.

@henrikbjorn

This comment has been minimized.

Show comment
Hide comment
@henrikbjorn

henrikbjorn Nov 5, 2015

Contributor

@weaverryan assume i have several routing files i want added, if both have the same prefix i assume the latter will override the former?

Contributor

henrikbjorn commented Nov 5, 2015

@weaverryan assume i have several routing files i want added, if both have the same prefix i assume the latter will override the former?

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Nov 5, 2015

Member

@henrikbjorn it uses the same mechanism as normal route importing. So if you import 5 routes from admin.yml and 5 routes from admin2.yml - both with the prefix /admin, and you will end up with 10 routes. The only time later routes would override earlier routes is if they have the same route name - the prefix won't matter with that.

Member

weaverryan commented Nov 5, 2015

@henrikbjorn it uses the same mechanism as normal route importing. So if you import 5 routes from admin.yml and 5 routes from admin2.yml - both with the prefix /admin, and you will end up with 10 routes. The only time later routes would override earlier routes is if they have the same route name - the prefix won't matter with that.

@wouterj

This comment has been minimized.

Show comment
Hide comment
@wouterj

wouterj Nov 5, 2015

Member

Maybe we should allow mounting without a prefix?

Member

wouterj commented Nov 5, 2015

Maybe we should allow mounting without a prefix?

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Nov 5, 2015

Member

Before I answer that, what is the problem with the current implementation? :) I have some small things that I don't think are necessarily perfect, but I only want to change things if people like @henrikbjorn say "Hey, I was confused by X, it would have been more clear if I could just call Y".

Member

weaverryan commented Nov 5, 2015

Before I answer that, what is the problem with the current implementation? :) I have some small things that I don't think are necessarily perfect, but I only want to change things if people like @henrikbjorn say "Hey, I was confused by X, it would have been more clear if I could just call Y".

@wouterj

This comment has been minimized.

Show comment
Hide comment
@wouterj

wouterj Nov 5, 2015

Member

Let's describe what I think (as a non-native english speaker) the discussed function names mean:

  • mount() bind 2 things (first and second argument) together
  • import() includes something into the current stack

It turns out mount() doesn't mount argument 1 and 2, it mounts argument 2 in the current route collection with an optional prefix defined by 1. If I didn't knew the Yaml syntax behind this (a resource with a prefix option), I would actually think that I cannot bind 2 different route collections to the same mountpoint.

Furthermore, it turns out import actually doesn't do anything more than loading and returning what it just loaded. The thing you loaded now has to be attached/mounted to the main route collection.

Member

wouterj commented Nov 5, 2015

Let's describe what I think (as a non-native english speaker) the discussed function names mean:

  • mount() bind 2 things (first and second argument) together
  • import() includes something into the current stack

It turns out mount() doesn't mount argument 1 and 2, it mounts argument 2 in the current route collection with an optional prefix defined by 1. If I didn't knew the Yaml syntax behind this (a resource with a prefix option), I would actually think that I cannot bind 2 different route collections to the same mountpoint.

Furthermore, it turns out import actually doesn't do anything more than loading and returning what it just loaded. The thing you loaded now has to be attached/mounted to the main route collection.

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Nov 5, 2015

Member

About mount(), it's exactly as it is in Silex (http://silex.sensiolabs.org/doc/organizing_controllers.html). So if it hasn't been a problem there for people, we can assume it won't be a problem here (if it has been a problem, we should't repeat it).

About import(), I share your concerns. However, if you automatically put the new routes into the RouteCollectionBuilder, then you're not able to modify then further - e.g. add defaults or requirements to the routes in just that collection. Again, this idea comes from Silex (http://silex.sensiolabs.org/doc/providers.html#controller-providers), though it's a bit more obvious there, because you create new class, so it's obvious that it's not being imported until you actually add it to your app. Is it as simple as calling import() something else? Or making import() actually import and adding a less-used method that returns the new RouteCollectionBuilder without mounting it?

Member

weaverryan commented Nov 5, 2015

About mount(), it's exactly as it is in Silex (http://silex.sensiolabs.org/doc/organizing_controllers.html). So if it hasn't been a problem there for people, we can assume it won't be a problem here (if it has been a problem, we should't repeat it).

About import(), I share your concerns. However, if you automatically put the new routes into the RouteCollectionBuilder, then you're not able to modify then further - e.g. add defaults or requirements to the routes in just that collection. Again, this idea comes from Silex (http://silex.sensiolabs.org/doc/providers.html#controller-providers), though it's a bit more obvious there, because you create new class, so it's obvious that it's not being imported until you actually add it to your app. Is it as simple as calling import() something else? Or making import() actually import and adding a less-used method that returns the new RouteCollectionBuilder without mounting it?

@wouterj

This comment has been minimized.

Show comment
Hide comment
@wouterj

wouterj Nov 5, 2015

Member

We can rename it to load() or import the route collection and return the imported route collection, allowing to change the instance. (this is done with registering Container definitions for instance)

Member

wouterj commented Nov 5, 2015

We can rename it to load() or import the route collection and return the imported route collection, allowing to change the instance. (this is done with registering Container definitions for instance)

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Nov 5, 2015

Member

@wouterj you're right - I didn't think about that - that's great :). See #16477.

Member

weaverryan commented Nov 5, 2015

@wouterj you're right - I didn't think about that - that's great :). See #16477.

@aschempp aschempp referenced this pull request Nov 6, 2015

Closed

Use the MicroKernel (maybe) #8

@henrikbjorn

This comment has been minimized.

Show comment
Hide comment
@henrikbjorn

henrikbjorn Nov 6, 2015

Contributor

@weaverryan i think the confusing part for me was the $prefix in mount is the first argument, and i would the assume it would be mandatory and therefor override my ealier prefix. But if the argument order was different eg mount($routes, $prefix = null) then i would make sense.

But that would proberly be a bigger BC break than your proposed solution.

Also a lot of the Bundles seems they are geared very much at usage in a directory structure that is the same as the standard edition which is a shame (eg app/Resources/views)

edit: my playground thingy https://github.com/henrikbjorn/Muse

Contributor

henrikbjorn commented Nov 6, 2015

@weaverryan i think the confusing part for me was the $prefix in mount is the first argument, and i would the assume it would be mandatory and therefor override my ealier prefix. But if the argument order was different eg mount($routes, $prefix = null) then i would make sense.

But that would proberly be a bigger BC break than your proposed solution.

Also a lot of the Bundles seems they are geared very much at usage in a directory structure that is the same as the standard edition which is a shame (eg app/Resources/views)

edit: my playground thingy https://github.com/henrikbjorn/Muse

@janit

This comment has been minimized.

Show comment
Hide comment
@janit

janit Nov 6, 2015

All I see is references to 2.8. Just verifying that this will this be in 3.0 too?

janit commented Nov 6, 2015

All I see is references to 2.8. Just verifying that this will this be in 3.0 too?

@Pierstoval

This comment has been minimized.

Show comment
Hide comment
@Pierstoval

Pierstoval Nov 6, 2015

Contributor

It will be merged in the next "merge 2.8 on 3.0" I guess :)

Contributor

Pierstoval commented Nov 6, 2015

It will be merged in the next "merge 2.8 on 3.0" I guess :)

@SoboLAN

This comment has been minimized.

Show comment
Hide comment
@SoboLAN

SoboLAN Nov 8, 2015

@fabpot I see this was merged in 2.8 branch. But 2.8 requires PHP 5.3.9, while traits are only available in PHP 5.4+ .

Are you sure it's correct to have this in 2.8 ?

SoboLAN commented Nov 8, 2015

@fabpot I see this was merged in 2.8 branch. But 2.8 requires PHP 5.3.9, while traits are only available in PHP 5.4+ .

Are you sure it's correct to have this in 2.8 ?

@wouterj

This comment has been minimized.

Show comment
Hide comment
@wouterj

wouterj Nov 8, 2015

Member

@SoboLAN this is an additional feature. Symfony doesn't require you to use this feature, so the minimum requirement is still 5.3.9.

If you want to use this feature, you have to use PHP 5.4.

Member

wouterj commented Nov 8, 2015

@SoboLAN this is an additional feature. Symfony doesn't require you to use this feature, so the minimum requirement is still 5.3.9.

If you want to use this feature, you have to use PHP 5.4.

@SoboLAN

This comment has been minimized.

Show comment
Hide comment
@SoboLAN

SoboLAN Nov 8, 2015

@wouterj You could say that about most of the functionality provided by Symfony.

If a minimum PHP version is specified, then I expect that absolutely everything will run without problems if I use Symfony on that PHP version (or newer). That's the whole point of why Composer allows specifying something like this.

I think it's a very reasonable assumption. Why ? Because everyone who has some restrictions on production systems (and most everyone has) and needs to decide what Symfony version to use, will rely on that 1 line in composer.json (which specifies minimum PHP version) to make his decision. It is the single most important factor in these kind of decisions.

Not respecting that is kind of alarming for me because I can never know what to expect. I would constantly ask myself:

If I use this method in my project, will it work in production servers ? (where I have a locked and usually older PHP version than on my dev machine)

If you (the deciders of Symfony) strongly believe that it should be like this, then fine. Either way I have no authority to dictate anything around here. But think about it: if you do this now, then where do you draw the line ? If you go down this path, where do you stop ?

I think that it's a dangerous precedent because it takes away all the confidence in that requires PHP version X statement.

Without that confidence, what is a developer or devops going to do ? What if he has a big Symfony project (or 20 projects) in his production systems ? Test absolutely every functionality when he makes a routine patch update (e.g. 2.6.9 -> 2.6.10) ? Just to be sure that what is written in composer.json is not a lie and that his website won't blow up ?

SoboLAN commented Nov 8, 2015

@wouterj You could say that about most of the functionality provided by Symfony.

If a minimum PHP version is specified, then I expect that absolutely everything will run without problems if I use Symfony on that PHP version (or newer). That's the whole point of why Composer allows specifying something like this.

I think it's a very reasonable assumption. Why ? Because everyone who has some restrictions on production systems (and most everyone has) and needs to decide what Symfony version to use, will rely on that 1 line in composer.json (which specifies minimum PHP version) to make his decision. It is the single most important factor in these kind of decisions.

Not respecting that is kind of alarming for me because I can never know what to expect. I would constantly ask myself:

If I use this method in my project, will it work in production servers ? (where I have a locked and usually older PHP version than on my dev machine)

If you (the deciders of Symfony) strongly believe that it should be like this, then fine. Either way I have no authority to dictate anything around here. But think about it: if you do this now, then where do you draw the line ? If you go down this path, where do you stop ?

I think that it's a dangerous precedent because it takes away all the confidence in that requires PHP version X statement.

Without that confidence, what is a developer or devops going to do ? What if he has a big Symfony project (or 20 projects) in his production systems ? Test absolutely every functionality when he makes a routine patch update (e.g. 2.6.9 -> 2.6.10) ? Just to be sure that what is written in composer.json is not a lie and that his website won't blow up ?

@wouterj

This comment has been minimized.

Show comment
Hide comment
@wouterj

wouterj Nov 8, 2015

Member

There are many packages in the suggest sections of Symfony's framework, components and bundles. These are all libraries that have some kind of integration in the component/bundle, but aren't required for the main usage (for instance, doctrine/annotations is required when using annotations with the Validator component, but as you can use Yaml/Xml/PHP, it's in the suggest section).

Also, 95% of the people that are going to use Symfony are not going to use this trait. It's a really nice feature, but more to advocate Symfony ("Symfony isn't that massive huge framework you think it is!") and for some people that are not going to base on top of the Standard Edition (and most people installing 2.8 will stick with the Standard Edition).

At last, it's very clear (or debuggable within 30 seconds) that this feature requires PHP 5.4 as you need to use the trait yourself. A developer using this feature on a PHP 5.3 server will either know the trait feature and identifies it as something that requires PHP 5.4 or will get an error message, search for "php trait" on google and find out that it requries PHP 5.4. Furthermore, I'm sure the documentation about this feature will mention the PHP 5.4 requirement "above the fold" of the article.

So for these reasons, I think it's safe to put this PHP-5.4-requiring-feature in Symfony 2.8. I don't think you need to be afraid that Symfony will cross the line many times in the future. The Symfony framework has a core team that's very focussed on it's usage (with the BC promise, easy upgrade paths, etc. as results).

At last, I want to thank you for sharing your worries in such a clear and long comment!

Member

wouterj commented Nov 8, 2015

There are many packages in the suggest sections of Symfony's framework, components and bundles. These are all libraries that have some kind of integration in the component/bundle, but aren't required for the main usage (for instance, doctrine/annotations is required when using annotations with the Validator component, but as you can use Yaml/Xml/PHP, it's in the suggest section).

Also, 95% of the people that are going to use Symfony are not going to use this trait. It's a really nice feature, but more to advocate Symfony ("Symfony isn't that massive huge framework you think it is!") and for some people that are not going to base on top of the Standard Edition (and most people installing 2.8 will stick with the Standard Edition).

At last, it's very clear (or debuggable within 30 seconds) that this feature requires PHP 5.4 as you need to use the trait yourself. A developer using this feature on a PHP 5.3 server will either know the trait feature and identifies it as something that requires PHP 5.4 or will get an error message, search for "php trait" on google and find out that it requries PHP 5.4. Furthermore, I'm sure the documentation about this feature will mention the PHP 5.4 requirement "above the fold" of the article.

So for these reasons, I think it's safe to put this PHP-5.4-requiring-feature in Symfony 2.8. I don't think you need to be afraid that Symfony will cross the line many times in the future. The Symfony framework has a core team that's very focussed on it's usage (with the BC promise, easy upgrade paths, etc. as results).

At last, I want to thank you for sharing your worries in such a clear and long comment!

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Nov 8, 2015

Member

I agree with Wouter - this is a very special situation - so don't worry @SoboLAN, we take this thing quite seriously.

Btw, nothing depends on this class in Symfony, it's a total extra feature. If you want it on 5.3, you can safely copy this class and tweak it to work with 5.3.

Cheers!

Member

weaverryan commented Nov 8, 2015

I agree with Wouter - this is a very special situation - so don't worry @SoboLAN, we take this thing quite seriously.

Btw, nothing depends on this class in Symfony, it's a total extra feature. If you want it on 5.3, you can safely copy this class and tweak it to work with 5.3.

Cheers!

@Pierstoval

This comment has been minimized.

Show comment
Hide comment
@Pierstoval

Pierstoval Nov 8, 2015

Contributor

Plus, to add another argument, this feature is mostly for new projects, and I doubt that developers creating new projects will use an old PHP version.

Contributor

Pierstoval commented Nov 8, 2015

Plus, to add another argument, this feature is mostly for new projects, and I doubt that developers creating new projects will use an old PHP version.

@psampaz

This comment has been minimized.

Show comment
Hide comment
@psampaz

psampaz Nov 8, 2015

Contributor

I totally agree with @SoboLAN.

Contributor

psampaz commented Nov 8, 2015

I totally agree with @SoboLAN.

@javiereguiluz

This comment has been minimized.

Show comment
Hide comment
@javiereguiluz

javiereguiluz Nov 8, 2015

Member

@wouterj @weaverryan what you say is true and reasonable. But at the same time I find @SoboLAN's comments very reasonable too. If some code claims 5.3 compatibility, I understad that it's for its entire codebase.

Member

javiereguiluz commented Nov 8, 2015

@wouterj @weaverryan what you say is true and reasonable. But at the same time I find @SoboLAN's comments very reasonable too. If some code claims 5.3 compatibility, I understad that it's for its entire codebase.

@Pierstoval

This comment has been minimized.

Show comment
Hide comment
@Pierstoval

Pierstoval Nov 8, 2015

Contributor

gedmo/doctrine-extensions has a PHP requirement of >=5.3.2 and it has traits. If you don't want to use them, you don't have to, and your code won't break.

It's been already said:

  • 95% of the people that are going to use Symfony are not going to use this trait
  • nothing depends on this class in Symfony, it's a total extra feature
  • this feature is mostly for new projects

For me, these reasons are enough to keep it in the 2.8 branch

Contributor

Pierstoval commented Nov 8, 2015

gedmo/doctrine-extensions has a PHP requirement of >=5.3.2 and it has traits. If you don't want to use them, you don't have to, and your code won't break.

It's been already said:

  • 95% of the people that are going to use Symfony are not going to use this trait
  • nothing depends on this class in Symfony, it's a total extra feature
  • this feature is mostly for new projects

For me, these reasons are enough to keep it in the 2.8 branch

@Tobion

This comment has been minimized.

Show comment
Hide comment
@Tobion

Tobion Nov 8, 2015

Member

Btw we already had traits in symfony before (ContainerAwareTrait). Not sure what the goal of this discussion is. Nobody forces you to use it.

Member

Tobion commented Nov 8, 2015

Btw we already had traits in symfony before (ContainerAwareTrait). Not sure what the goal of this discussion is. Nobody forces you to use it.

@SoboLAN

This comment has been minimized.

Show comment
Hide comment
@SoboLAN

SoboLAN Nov 10, 2015

@Pierstoval @Tobion I totally understand what you're saying. No one is making you use these features.

But the problem in these situations is that you may use some classes/functionalities without knowing it.

Let's take a simple example and say that I have the following line in one of my Controllers:

$this->get('validator')->validate($someObject, array('group1', 'group2'));

I'm not using what I'm not supposed to. But, behind the scenes, how much is going on in order to validate that object ? You have to:

  • build the validator object (which is not very simple)
  • let's say read a validation.yml file
  • interpret the file
  • have the Validator go through it and determine which constraints contain those specified groups
  • apply those constraints
  • build violation errors
  • etc.

There are probably thousands of lines of code being executed inside that 1 line from above.

If somewhere in all that code you add code (like for example a trait) that is not compatible with what PHP version you specify in composer, then my project will maybe fail, usually in ways that can be difficult to reproduce.

@Pierstoval @Tobion I only presented above the reason for why this whole discussion started.

@wouterj assured me that something like this would never happen. I hope he's right. I'm very familiar with Symfony's promise to have excellent backwards-compatibility. I will just have faith that bad scenarios like the one above will never happen :) .

I hope I did not piss people off with my concerns. If I did, it was not my intention. I was just somewhat worried.

SoboLAN commented Nov 10, 2015

@Pierstoval @Tobion I totally understand what you're saying. No one is making you use these features.

But the problem in these situations is that you may use some classes/functionalities without knowing it.

Let's take a simple example and say that I have the following line in one of my Controllers:

$this->get('validator')->validate($someObject, array('group1', 'group2'));

I'm not using what I'm not supposed to. But, behind the scenes, how much is going on in order to validate that object ? You have to:

  • build the validator object (which is not very simple)
  • let's say read a validation.yml file
  • interpret the file
  • have the Validator go through it and determine which constraints contain those specified groups
  • apply those constraints
  • build violation errors
  • etc.

There are probably thousands of lines of code being executed inside that 1 line from above.

If somewhere in all that code you add code (like for example a trait) that is not compatible with what PHP version you specify in composer, then my project will maybe fail, usually in ways that can be difficult to reproduce.

@Pierstoval @Tobion I only presented above the reason for why this whole discussion started.

@wouterj assured me that something like this would never happen. I hope he's right. I'm very familiar with Symfony's promise to have excellent backwards-compatibility. I will just have faith that bad scenarios like the one above will never happen :) .

I hope I did not piss people off with my concerns. If I did, it was not my intention. I was just somewhat worried.

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Nov 10, 2015

Member

@SoboLAN Symfony does not rely on the MicroKernelTrait, which means it's true that Symfony works with 5.3. No project is going to break because we added the trait.
If someone or some lib start relying on the trait to provide some feature, then it's their responsibility to enforce the correct minimum PHP version.

Member

nicolas-grekas commented Nov 10, 2015

@SoboLAN Symfony does not rely on the MicroKernelTrait, which means it's true that Symfony works with 5.3. No project is going to break because we added the trait.
If someone or some lib start relying on the trait to provide some feature, then it's their responsibility to enforce the correct minimum PHP version.

@fabpot fabpot referenced this pull request Nov 16, 2015

Merged

Release v2.8.0-BETA1 #16550

fabpot added a commit that referenced this pull request Nov 23, 2015

bug #16477 [Routing] Changing RouteCollectionBuilder::import() behavi…
…or to add to the builder (weaverryan)

This PR was squashed before being merged into the 2.8 branch (closes #16477).

Discussion
----------

[Routing] Changing RouteCollectionBuilder::import() behavior to add to the builder

| Q             | A
| ------------- | ---
| Bug fix?      | behavior change
| New feature?  | behavior change
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Based on conversation starting here: #15990 (comment).

```php
// Before:
$routes->mount('/admin', $routes->import(__DIR__.'/config/admin.yml');

// After:
$routes->import(__DIR__.'/config/admin.yml', '/admin');
```

This makes `import()` actually add the `RouteCollectionBuilder` into itself. We didn't do this before at Fabien's request, and actually the current implementation (before this PR) is quite "clean". However, I agree with @wouterj that `import()` really sounds/looks like it will actually *import* those routes *into* this `RouteCollectionBuilder`.

This change is subjective - we just need to pick which way we like better and run full steam with it.

Commits
-------

8feb9ef [Routing] Changing RouteCollectionBuilder::import() behavior to add to the builder

fabpot added a commit to symfony/routing that referenced this pull request Nov 23, 2015

bug #16477 [Routing] Changing RouteCollectionBuilder::import() behavi…
…or to add to the builder (weaverryan)

This PR was squashed before being merged into the 2.8 branch (closes #16477).

Discussion
----------

[Routing] Changing RouteCollectionBuilder::import() behavior to add to the builder

| Q             | A
| ------------- | ---
| Bug fix?      | behavior change
| New feature?  | behavior change
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Based on conversation starting here: symfony/symfony#15990 (comment).

```php
// Before:
$routes->mount('/admin', $routes->import(__DIR__.'/config/admin.yml');

// After:
$routes->import(__DIR__.'/config/admin.yml', '/admin');
```

This makes `import()` actually add the `RouteCollectionBuilder` into itself. We didn't do this before at Fabien's request, and actually the current implementation (before this PR) is quite "clean". However, I agree with @wouterj that `import()` really sounds/looks like it will actually *import* those routes *into* this `RouteCollectionBuilder`.

This change is subjective - we just need to pick which way we like better and run full steam with it.

Commits
-------

8feb9ef [Routing] Changing RouteCollectionBuilder::import() behavior to add to the builder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment