[DI] Add prototype services for PSR4-based discovery and registration #21289

nicolas-grekas · 2017-01-14T12:03:41Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	to be done

Talking with @dunglas, we wondered if this could be a good idea, as a more general approach to folder-based service registration as done in DunglasActionBundle.

So here is the implementation.

This allows one to define a set of services as such:

services:
    App\:
        resource: ../src/{Controller,Command} # relative to the current file as usual
        autowire: true # or any other attributes really

This looks for php files in the "src" folder, derivates PSR-4 class names from them, and uses class_exists for final discovery. The resulting services are named after the classes found this way.

The "resource" attribute can be a glob to select only a subset of the classes/files.

This approach has several advantages over DunglasActionBundle:

it is resilient to missing parent classes (see test case)
it loads classes using the normal code path (the autoloader), thus play well with them (e.g. if one registered a special autoloader).
it is more predictable, because it uses discovered paths as the only source of ids/classes to register - vs relying on get_declared_classes, which would make things context sensitive.

Fits well with current initiatives to me.

dunglas · 2017-01-14T12:26:30Z

src/Symfony/Component/DependencyInjection/Loader/FileLoader.php

+        $directory = $this->locator->locate($directory, $this->currentDir, true);
+        $directory = realpath($directory) ?: $directory;
+        if (!$classes = $this->findClasses($psr4Glob, $directory)) {
+            throw new InvalidArgumentException(sprintf('No classes matching "%s" were found in directory "%s".', $psr4Glob, $directory));


Shouldn't we log something instead of throwing? It will allow to register empty directories (useful for starter packs for instance).

dunglas · 2017-01-14T12:28:15Z

src/Symfony/Component/DependencyInjection/Loader/FileLoader.php

+
+            $psr4Prefix = $match[0];
+            $classRegexp = self::patternToRegexp($psr4Glob);
+            $excludeTest = false === strpos($psr4Glob, 'Test', strlen($psr4Prefix));


Do we really want to hardcode such behaviors? If the user doesn't want to include the Test/ he can already register all subdirectories manually.

I think we do, because it gives a good default: test folder/classes usually contain strange things that can break PHP (look at our own test suite - this is were I needed this default first).

dunglas · 2017-01-14T12:34:22Z

src/Symfony/Component/DependencyInjection/Loader/FileLoader.php

+        }
+    }
+
+    private static function patternToRegexp($pattern)


If the method is private and called only from non-static methods, there is no benefit to make it static.

dunglas · 2017-01-14T13:52:07Z

👍

sstok · 2017-01-14T15:03:46Z

Does this also support Symfony specific conventions like parameters and the @Bundle notation?

GuilhemN · 2017-01-14T15:14:37Z

src/Symfony/Component/DependencyInjection/Loader/FileLoader.php

+        parent::setCurrentDir($dir);
+    }
+
+    public function registerClasses(Definition $prototype, $psr4Glob, $directory)


Should be protected, right?
I also suggest to make it final or @internal as I don't think it is useful to extend it.

Should be usable by people using the PhpFileLoader.

Ok, good point.

About final, I don't adhere to the trend of adding it everywhere, so no :)

Imo final methods are great because of the incoming 7.0 type hints: it will allow us to upgrade a bigger part of the codebase.
However, if this RFC is voted, it will be less useful.

I seriously hope that Parameter Type Widening is not performed when strict-types are used (in the child class file) 😑 this would be a big mistake otherwise.

nicolas-grekas · 2017-01-14T16:46:45Z

@sstok should be yes.

GuilhemN · 2017-01-14T15:23:58Z

src/Symfony/Component/DependencyInjection/Loader/FileLoader.php

@@ -23,6 +27,7 @@
 abstract class FileLoader extends BaseFileLoader
 {
    protected $container;
+    protected $currentDir;


private done

wouterj · 2017-01-14T17:22:29Z

I voted 😕, so let's explain some of my thoughts about this feature:

To start of: Symfony 3.3 is working at the very edge between the "magic convention-based Laravel" and "explicit and clear Symfony". All added features are both exciting and very scary because of their magic. Yet, I consider all currently merged features to still be explicit enough to overrule the "magic".
This PR however is removing quite a lot of explicitness. Previously, whenever someone asks on IRC how to find out which services they can use or how servcies are defined, I suggest them to take a look at the Resources/config directory of a bundle. All service definitions could be found somewhere in there. Just a Ctrl + F for the service ID or classname would give you all information you need.
With this PR however, all this information will be removed from the service configuration. Symfony will derive this information itself based on some magic globs.
From my experience, class discovery in projects is one of the most magic things in live. It's mostly based on conventions and often isn't easy to understand. Also, these conventions are prone to change, which can lead to problems with Symfony versions (see what happend with PHPspec when PSR-4 became a thing).
I think the current configuration isn't clear. Defining the psr-4 glob as service ID confused me a lot when looking at this PR. I would rather expect the PSR-4 glob to be define by psr4 and have a special resource or the like option to define the directory. Like:
```
services:
    app.controllers:
        psr4: AppBundle\Controller\
        resource: '@AppBundle/Controller/`
```
That said, I think it would make more sense to support this in a new loader:
```
imports:
    - { resource: ../src/Controller, psr4_prefix: AppBundle\Controller\ }
```
This seems more clear to me than including this in the service definition configuration.
I don't see much use-cases for this kind of 'include every class as service' thing. Yet, it does make some sense for e.g. automatic twig extension registration.
I'm worrying that this feature will lead to structuring your code based on container import conventions. For instance, I expect lots of people to start introducing Service namespaces in their bundles to register all classes in this namespace as services. That would be a very bad practice imo.
I'm not sure about excluding Test, I don't know why we should introduce such convention. Nobody should ever define a complete bundle using this method and it makes sense to sometimes include some test helper classes (which are often found in the Test sub-namespace).

nicolas-grekas · 2017-01-14T17:48:31Z

Symfony 3.3 is working at the very edge between the "magic convention-based Laravel"

and we have to account for the success of Laravel because of its DXness.
But I don't agree with the word "magic".
Magic means "I didn't ask for and it works".
Black (bad) magic means "some global context leaks in to provide this thing".

Here, it's plain explicit - and plain local.
You ask for something by writting this snippet. You get it.

Symfony will derive this information itself based on some ~~magic~~ globs.

You asked for it. No convention here.

services:
app.controllers:
psr4: AppBundle\Controller
resource: '@AppBundle/Controller/`

that does not work, because what matters is to be able to express a pattern that can be expanded to generate service ids.

imports:
- { resource: ../src/Controller, psr4_prefix: AppBundle\Controller\ }

same here.

I don't see much use-cases for this kind

Ask DunglasActionBundle supporters (221 stars, not bad).

I'm not sure about excluding Test, I don't know why we should introduce such convention. Nobody should ever define a complete bundle using this method

I think it's a good default - but if I can't convince the majority, I'll remove it (but I still think that a good default and that not excluding it will prove soon to lead to more problems than WTFs :) )

and it makes sense to sometimes include some test helper classes (which are often found in the Test sub-namespace).

Sure it does! It's really easy to opt-in "Test" classes.

Last word on your other comments about your feelings:

I do understand them.
I felt the same when autowiring was introduced.
Now?
I love it.
:)

wouterj · 2017-01-14T18:11:25Z

@nicolas-grekas please note that I'm 😕 and not 👎. I'm just not sure whether I like this or not, but I just wanted to share my current thoughts.

GuilhemN · 2017-01-14T22:51:21Z

src/Symfony/Component/DependencyInjection/Loader/FileLoader.php

+                        $classes[] = $class;
+                        continue;
+                    }
+                    if (!function_exists('opcache_is_script_cached') || !opcache_is_script_cached($info->getRealPath() ?: $path)) {


wouldn't something rejected by this check, be also rejected by the class_exists below?

having a script in cache does not mean there is a class inside - so that's not the same

theofidry · 2017-01-15T14:55:36Z

@wouterj a few thoughts:

This PR however is removing quite a lot of explicitness. Previously, whenever someone asks on IRC how to find out which services they can use or how servcies are defined, I suggest them to take a look at the Resources/config directory of a bundle

That's not true: a service can easily be added via a Compiler pass, and a few classes a service locators which are not declared as services, e.g. Controllers and (console) Commands.

The reason why DunglasApiBundle is popular, is that it allows you to remove those service locators (Controllers & Commands) and make use of dependency injection, without introducing the pain of having to declare them as services (although you do add the necessity to ensure your autowiring bindings are correct).

That said, I think it would make more sense to support this in a new loader:

imports:
    - { resource: ../src/Controller, psr4_prefix: AppBundle\Controller\ }

I must say that it makes more sense to me as well, with psr4_prefix which is optional. If it's already declared in the composer.json I don't see why (from a user perspective) you would need to declare it here as well. It seems more confusing and error prone than anything to me.

Maybe you would need to add a type:

imports:
    - { resource: ../src/Controller, type: autowired-services }

nicolas-grekas · 2017-01-15T19:23:15Z

If it's already declared in the composer.json I don't see why (from a user perspective) you would need to declare it

that would be black magic as defined earlier: some global context leaks in!

type: autowired-services

that is strong coupling with autowiring. This feature is orthogonal: autowiring is not required by this PR at all. The fact that it plays well with autowiring is not an unwanted side effect of course, but things are better decoupled+composed than strongly tightened together.

theofidry · 2017-01-15T19:33:40Z

So the few points lost in the edit:

What happens when you come across a file which is not a class: a function, a trait, an abstract class, an interface...
The include works with globs is that right? Is it possible to have a kinda granular filtering/include? We had a discussion with @dunglas about it somewhere at some point. For example you may have a class in the directory which should not be declared as a service or something
What happens if a class found is already registered as a service? (I would expect the original definition not to be touched)
I fall in the case several times where I have to do slightly more than "just" registering the services found in a directory, adding a special tag which the name can be determined from the FQCN. Is there any way to re-use that [the one from your PR] logic to achieve that rather than rolling a kinda duplicate of DunglasApiBundle for this specific task?

A few other things lost but can't remember them...

nicolas-grekas · 2017-01-15T20:14:30Z

What happens when you come across a file which is not a class: a function, a trait, an abstract class, an interface...

the code uses class_exists, which is selective about classes, so interfaces and traits are excluded. For abstract classes, they would be registered, but of course you'll get an error if you use them. Still, this will prove usefull when getter injection will be merged (because it allows to make them usable when abstract is only about getters).

The include works with globs is that right?

yes, and for consistency with #21270, I'm going to use a real glob call.
but I don't think we should add some "exclude" mechanism: if you happen to register too many classes, that's not an issue. Just don't use them. Or don't use this mechanism. Or, last option, mark these as private and all the non used ones will be removed automatically.

What happens if a class found is already registered as a service?

same logic as today: the last registered ones wins. Which means you should put these auto-registering declarations before the typical ones and you'll get the behavior you describe.

I fall in the case several times where I have to do slightly more than "just" registering the services found in a directory, adding a special tag which the name can be determined from the FQCN.

That's the reason why this needs to be in the "services" section and not in "imports":
the "psr4" attribute is just one amongst all the regular ones, and _default also applies.

eg this works:

services:
    App\Command\:
        psr4: ../src/Command
        tags: [console.command]

this will tag all discovered classes with the console.command tag

DunglasActionBundle provides one additional feature that this PR does not try to implement:
conditional tagging (the bundle allows to declare: every instance of EventSubscriberInterface should have the kernel.event_subscriber tag).
We've some ideas with @dunglas about this, but this is orthogonal to this PR, so may come in a later PR.

theofidry · 2017-01-15T21:05:38Z

@nicolas-grekas thanks for the answers.

Overall, I'm in favour of this mechanism: there is parts on my application where loosening the control whilst still using dependency injection over service locators which I prefer. There is however a big caveat which I'm not happy at all with: side effects.

It may not be clear for the typical user what happens when service IDs collides, i.e. which trumps on the other. (I think for this one a mention in the doc is enough)
You may have classes you don't expect to be registered which are. You're answer to this for now is "it's ok, if you don't use it they are going to be removed or it doesn't matter much". Whilst I perfectly understand the approach, IMO this is a big deal and this is by far the thing I hate the most in Laravel. (Further explanation in this below).

To extend on the second point, in Laravel you can try to retrieve any class from the DIC. It may not succeed, but it will try at least, even if the class in question is not meant to be registered in the DIC. Now in Symfony we are adopting a similar thing with this mechanism, more scoped as we are specifying the folders we want to apply this mechanism to, but the problem remains the same: you are registering things that should not be registered.

To this problem, I think we should, at least, proving the tooling necessary for the people how mind it like me to "uncover" this side-effect. For example a command listing the services registered in this way.

In case this seems weird to you I'm giving you the use case I have in mind: I get my hand on a application I'm not familiar with, the glob is hard to understand or I suspect a class is registered when it should not or simply, I want to see if what I expect to be registered actually is, how can I see that easily?

That's the reason why this needs to be in the "services" section and not in "imports":
the "psr4" attribute is just one amongst all the regular ones, and _default also applies.

Fair enough in a very simple case, but say mind the tag name needs a bit of processing to be determined (and is currently done in a Compiler pass which does something a la DunglasApiBundle). To extend a bit on this use case: it seems much more interesting to me, more than this PR to be honest, interesting to provide a way to re-use that logic of "find recursively the classes here, return an array of FQC => serviceDefinition", to which you could hook something to do whatever kind of processing you would like to in a compiler pass. Granted this may not be the scope of this PR, but I think it affects the way this feature is implemented (if we want to provide this kind of hooks).

theofidry · 2017-01-15T21:52:24Z

@nicolas-grekas I'm asking those questions here as I'm missing a bit the technical insight to be able to judge if this requires to be done upfront or if this can be done gradually, in which case this PR is just a first step. If you think this can be done gradually, I'm in favour to open dedicated issue for this PR and just 👍 this one, your call.

nicolas-grekas · 2017-01-16T09:56:14Z

It may not be clear for the typical user what happens when service IDs collides

That could also apply to "imports". That's why ordering is the only rule to me - for consistency.

in Laravel you can try to retrieve any class from the DIC

that is #18268 - discuss that there - nothing to do with this PR :)

mind the tag name needs a bit of processing to be determined

exactly the topic I mentionned before: DunglasActionBundle provides conditional tagging - and this PR does not try to do it, because that's something orthogonal - ie that must be done indenpendently, because it is not required for making this PR self consistent.

return an array of FQC => serviceDefinition

same here, that's building on top of this PR, or reusing some of its logic to do something else. Not required to make this PR self consistent.

All in all @theofidry, I'm really happy to see that this PR is raising many ideas in your mind.
Just let's things be built one consistent step at a time. PR after PR, each one self-consistent and useful by itself.

nicolas-grekas · 2017-01-16T10:10:33Z

PR updated, trying to listen to comments:

the glob pattern is moved from the "id" side to the "psr4" side, so that we can use the real glob() function
given that glob() is powerful enough to include/exclude dirs, I removed the Test special exclusion
the "id" side now must be a PSR4 namespace prefix, thus must end with a \ (same convention as composer)

nicolas-grekas · 2017-02-09T13:21:09Z

@fabpot comments addressed

stof · 2017-02-09T14:29:06Z

src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

@@ -659,13 +692,15 @@ private function loadFromExtensions(array $content)
     */
    private static function checkDefinition($id, array $definition, $file)
    {
+        $keywords = isset($definition['resources']) ? static::$prototypeKeywords : static::$serviceKeywords;


private properties must be accessed using self::, not static::. Otherwise, the code breaks when extending the class

stof · 2017-02-09T18:38:39Z

src/Symfony/Component/DependencyInjection/Loader/FileLoader.php

+        return $classes;
+    }
+
+    private function glob($resources, $recursive, &$prefixLen = null)


do we ever call it in non-recursive mode ? I see a single usage of this method

That's preparing for #21270

fabpot · 2017-02-12T12:07:33Z

src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_prototype.yml

@@ -0,0 +1,3 @@
+services:
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\:
+        resources: ../Prototype


I would have named it resource. A plural seems to indicate that the value is an array, which is not supported here.

fabpot · 2017-02-12T15:54:34Z

Tests are broken.

…ration

nicolas-grekas · 2017-02-12T22:14:55Z

back to green

fabpot · 2017-02-13T05:22:19Z

Thank you @nicolas-grekas.

@dunglas

…nd registration (nicolas-grekas) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Add prototype services for PSR4-based discovery and registration | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | to be done Talking with @dunglas, we wondered if this could be a good idea, as a more general approach to folder-based service registration as done in [DunglasActionBundle](https://github.com/dunglas/DunglasActionBundle/blob/master/DependencyInjection/DunglasActionExtension.php). So here is the implementation. This allows one to define a set of services as such: ```yaml services: App\: resources: ../src/{Controller,Command} # relative to the current file as usual autowire: true # or any other attributes really ``` This looks for php files in the "src" folder, derivates PSR-4 class names from them, and uses `class_exists` for final discovery. The resulting services are named after the classes found this way. The "resource" attribute can be a glob to select only a subset of the classes/files. This approach has several advantages over [DunglasActionBundle](https://github.com/dunglas/DunglasActionBundle/blob/master/DependencyInjection/DunglasActionExtension.php): - it is resilient to missing parent classes (see test case) - it loads classes using the normal code path (the autoloader), thus play well with them (e.g. if one registered a special autoloader). - it is more predictable, because it uses discovered paths as the only source of ids/classes to register - vs relying on `get_declared_classes`, which would make things context sensitive. Fits well with current initiatives to me. Commits ------- 03470b7 [DI] Add "psr4" service attribute for PSR4-based discovery and registration

TomasVotruba · 2017-04-28T15:20:32Z

Awesome job!

Finally can drop this bundle doing pretty much the same thing: https://github.com/Symplify/AutoServiceRegistration

@nicolas-grekas

…loader (weaverryan) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Fixing missing "exclude" functionality from PSR4 loader | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | none | License | MIT | Doc PR | TODO When the PSR4 loader was added in #21289, @nicolas-grekas said this: > given that glob() is powerful enough to include/exclude dirs, I removed the Test special exclusion (source: #21289 (comment)) But, I don't believe that's true! [Glob is all about inclusion, not exclusion](https://en.wikipedia.org/wiki/Glob_(programming)#Syntax) - the maximum you can exclude is a single character. Thus, I've marked this as a bug. My motivation came from upgrading KnpU to the new 3.3 DI stuff. We have many directories (40) in `src/`, and listing them all one-by-one in `resource:` is crazy - the `resource` line would be ~350 characters long and quite unreadable. What I really want to do is include *everything* and then exclude few directories. I tried to do this with `glob`, but it's not possible. This PR allows for something like this: ```yml services: _defaults: public: false # ... AppBundle\: resource: '../../src/AppBundle/*' exclude: '../../src/AppBundle/{AppBundle.php,Entity}' ``` This works *beautifully* in practice. And even if I forget to exclude a directory - since the services are private - **they're ultimately removed from the container anyways**. In fact, the *only* reason I need to exclude *anything* is because of the new "service argument resolver", which causes entities to not be properly removed from the compiled container. Thanks! Commits ------- 7d07f19 Allowing prototype/PSR4 service loading ability to exclude, because glob doesn't support this

nicolas-grekas added this to the 3.3 milestone Jan 14, 2017

carsonbot added Status: Needs Review DependencyInjection Feature labels Jan 14, 2017

nicolas-grekas force-pushed the di-batch branch from bf7018a to dbfac50 Compare January 14, 2017 12:37

dunglas reviewed Jan 14, 2017

View reviewed changes

nicolas-grekas force-pushed the di-batch branch 2 times, most recently from d4cc560 to 6584292 Compare January 14, 2017 13:40

GuilhemN reviewed Jan 14, 2017

View reviewed changes

nicolas-grekas force-pushed the di-batch branch from 6584292 to 4705112 Compare January 14, 2017 16:59

GuilhemN reviewed Jan 14, 2017

View reviewed changes

nicolas-grekas force-pushed the di-batch branch 3 times, most recently from 046d5b5 to 5389d98 Compare January 16, 2017 09:34

nicolas-grekas force-pushed the di-batch branch from 5389d98 to ccda044 Compare January 16, 2017 10:05

nicolas-grekas force-pushed the di-batch branch from eb69028 to 28420c2 Compare February 9, 2017 13:19

nicolas-grekas mentioned this pull request Feb 9, 2017

[Finder] Add double-star matching to Glob::toRegex() #21572

Merged

nicolas-grekas changed the title ~~[DI] Add "psr4" service attribute for PSR4-based discovery and registration~~ [DI] Add protype services for PSR4-based discovery and registration Feb 9, 2017

nicolas-grekas force-pushed the di-batch branch from 28420c2 to 4999035 Compare February 9, 2017 13:32

nicolas-grekas changed the title ~~[DI] Add protype services for PSR4-based discovery and registration~~ [DI] Add prototype services for PSR4-based discovery and registration Feb 9, 2017

nicolas-grekas force-pushed the di-batch branch from 4999035 to 75d66d8 Compare February 9, 2017 14:31

stof reviewed Feb 9, 2017

View reviewed changes

nicolas-grekas force-pushed the di-batch branch from 75d66d8 to 0564cd8 Compare February 10, 2017 15:03

nicolas-grekas added the Ready label Feb 11, 2017

fabpot approved these changes Feb 12, 2017

View reviewed changes

nicolas-grekas force-pushed the di-batch branch from 0564cd8 to 6b7892c Compare February 12, 2017 13:10

[DI] Add "psr4" service attribute for PSR4-based discovery and regist…

03470b7

…ration

nicolas-grekas force-pushed the di-batch branch from 6b7892c to 03470b7 Compare February 12, 2017 22:09

fabpot merged commit 03470b7 into symfony:master Feb 13, 2017

dunglas mentioned this pull request Feb 13, 2017

[FrameworkBundle] Introduce autowirable ControllerTrait #18193

Closed

nicolas-grekas deleted the di-batch branch February 14, 2017 16:21

javiereguiluz mentioned this pull request Feb 26, 2017

Add prototype services for PSR4-based discovery and registration symfony/symfony-docs#7544

Closed

theofidry mentioned this pull request Mar 15, 2017

[DI] PSR4 loader suggestions #22016

Closed

nicolas-grekas mentioned this pull request Apr 21, 2017

[DI] Enable factory attribute in service defaults #22189

Closed

This was referenced Apr 28, 2017

[DefaultAutoconfigure] init + apply Symfony 3.3 deprecated-packages/symplify#155

Merged

[DI] Introducing autoconfigure: automatic _instanceof configuration #22234

Closed

fabpot mentioned this pull request May 1, 2017

Release v3.3.0-BETA1 #22603

Merged

weaverryan mentioned this pull request May 9, 2017

[DI] Fixing missing "exclude" functionality from PSR4 loader #22680

Merged

[DI] Add prototype services for PSR4-based discovery and registration #21289

[DI] Add prototype services for PSR4-based discovery and registration #21289

Conversation

nicolas-grekas commented Jan 14, 2017 • edited Loading

dunglas Jan 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dunglas commented Jan 14, 2017

sstok commented Jan 14, 2017

GuilhemN Jan 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuilhemN Jan 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas commented Jan 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wouterj commented Jan 14, 2017

nicolas-grekas commented Jan 14, 2017 • edited Loading

wouterj commented Jan 14, 2017

Choose a reason for hiding this comment

nicolas-grekas Jan 16, 2017 • edited Loading

Choose a reason for hiding this comment

theofidry commented Jan 15, 2017 • edited by nicolas-grekas Loading

nicolas-grekas commented Jan 15, 2017 • edited Loading

theofidry commented Jan 15, 2017

nicolas-grekas commented Jan 15, 2017

theofidry commented Jan 15, 2017

theofidry commented Jan 15, 2017

nicolas-grekas commented Jan 16, 2017

nicolas-grekas commented Jan 16, 2017 • edited Loading

nicolas-grekas commented Feb 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Feb 12, 2017

nicolas-grekas commented Feb 12, 2017

fabpot commented Feb 13, 2017

TomasVotruba commented Apr 28, 2017 • edited Loading

nicolas-grekas commented Jan 14, 2017 •

edited

Loading

dunglas Jan 14, 2017 •

edited

Loading

GuilhemN Jan 14, 2017 •

edited

Loading

GuilhemN Jan 14, 2017 •

edited

Loading

nicolas-grekas commented Jan 14, 2017 •

edited

Loading

nicolas-grekas Jan 16, 2017 •

edited

Loading

theofidry commented Jan 15, 2017 •

edited by nicolas-grekas

Loading

nicolas-grekas commented Jan 15, 2017 •

edited

Loading

nicolas-grekas commented Jan 16, 2017 •

edited

Loading

TomasVotruba commented Apr 28, 2017 •

edited

Loading