[DI] Deprecate Container::set for nulls, initialized and alias services #19668

ro0NL · 2016-08-19T08:47:59Z

Q	A
Branch?	"master"
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	yes
Tests pass?	not yet
Fixed tickets	#19192
License	MIT
Doc PR	reference to the documentation PR, if any

Not sure what i got myself into, it's a true adventure so far. But this deprecates
Container::set('id', null) 😱 and Container::set('id', $previousSetService)

We violate the contract, which we are also fixing at the same time: [Bridge/Doctrine] Reset the EM lazy-proxy instead of the EM service #19203
- We can consider ContainerInterface::remove if we need to support it (so far im not sure really)
This somewhat relates to the ContainerBuilder - [DI] Deprecate Container::isFrozen and introduce isCompiled #19673, [DI] Allow getting synthetic before compilation #19715

Before/After:

set('id', null)
- Resets the service, but is deprecated as of 3.2
- In 4.0 we can remove the case (expecting it to be an object as documented) or throw. This is yet to be decided.
set('id', $instance)
- Allowed if, and only if the service is not initialized previously. This is recommended by design, and keeps the dependency graph trusty. Breaking this rule is deprecated as of 3.2.
- In 4.0 this should throw an exception.
set('alias', $instance)
- Set a new "alias" service, but is deprecated in 3.2
- In 4.0 this sets the aliased service, following the rule above

stof · 2016-08-19T08:54:25Z

src/Symfony/Component/DependencyInjection/Container.php

            ) {
                return true;
            }
+            if (array_key_exists($id, $this->services)) {
+                @trigger_error(sprintf('Checking for the existence of a unset "%s" service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);


this does not make sense, as there won't be any unset services in 4.0, and code calling has() has no way to know whether the service instance was reset or no.

And btw, you change the code to unset the array entry instead of setting it to null, so this if won't match anymore

hmm no, we were already unsetting it, so we already don't have null values in this array

The current behavior is messed up and untrusty. The property is protected (!). Lets be safe.

If we can go with private.. that be great, as of 4.0 this still can be considered a safeguard when sticking with protected.

sstok · 2016-08-19T08:58:16Z

I would say yes, because Container::set('id', null) only resets the pointed service but other services that depend on that service are not reset (which cause some unexpected situations and mismatch).

ro0NL · 2016-08-19T09:09:04Z

src/Symfony/Component/DependencyInjection/Container.php

-        $this->services[$id] = $service;
-
-        if (null === $service) {
+        if (null === $service) { // safeguard for 4.0


As long as we stick with protected $services (opposed to private) i tend to keep this safeguard in 4.0, for historic reasons i guess.

It would be better to remove this whole block (adhere to the doc, like always) and remove all array_key_exists thingy's in 4.0 (as those are safeguards too in a way, again due protected).

ro0NL · 2016-08-19T09:17:08Z

(ContainerBuilder) tests seem to be green 🎉

@nicolas-grekas WDYT?

ro0NL · 2016-08-19T10:39:31Z

Long story short; we should consider a BC-break marking $services (and preferably others) private, and cleanup. Do it now, or forever be sorry (not sure if we ~~can~~ want to bridge this following deprecations?).

stof · 2016-08-19T11:13:23Z

@ro0NL we cannot make it private, as the methods generated in the dumped container need to access this property

ro0NL · 2016-08-19T11:24:07Z

@stof makes sense. I still need to dig in this method generation, how that is affected. But first thought is to have a protected function getService that allows for read, but not write. Allowing us to go private?

ogizanagi · 2016-08-19T12:33:41Z

This deprecates setting an existing service to null (and thus disallow unsetting it, which is a good thing to me), but what about the original issue asking for deprecating setting a non-synthetic service (i.e: replacing an existing service with a new instance) ?

I think its easier to handle those cases in the very same PR. WDYT ?

ro0NL · 2016-08-19T12:37:05Z

Agree. Im not sure about the real problem though, hence i asked nicolas-grekas.

We could do typechecks, allow it only for the (exact) same type. But yeah, im curious how this is a real issue...

edit: from the ContainerBuilder::set side it could be an issue.. however, imo, that's not specific to replacing but generally mix&match synthetics and definitions.

ogizanagi · 2016-08-19T12:50:03Z

@ro0NL : This is an issue (from my POV) because of the reason stated by @sstok in #19668 (comment) and the fact that the container should probably not allow to be modified by any third party, appart from setting synthetic services.
Once a service is set, it should not be changed or unset. Any instantiated service relying on it will not be rebuilt, and will still hold a reference on an invalid dependency (the closed entity manager example with Doctrine).

As most services should be stateless, this should not be a major issue (resetting their state should be something handled by something else than the container (The manager registry in the Doctrine case)).

ro0NL · 2016-08-19T13:02:09Z

Got your point. I'm thinking too much registry-pattern. From the DI pov that makes really sense.

ogizanagi · 2016-08-19T13:29:49Z

src/Symfony/Component/DependencyInjection/Container.php

@@ -169,22 +170,25 @@ public function set($id, $service)
        }

        if (isset($this->aliases[$id])) {
-            unset($this->aliases[$id]);
+            @trigger_error(sprintf('Replacing the "%s" (alias of "%s") service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id, $this->aliases[$id]), E_USER_DEPRECATED);
+        } elseif (isset($this->services[$id])) {


I think isset is not enough to check if a previous service exists (see Container::has()).
But I don't know if we should allow replacing a non instantiated service or not.

AFAIK null/whatever can only be expected due the fact of protected $services, or what am i missing here? Ie. set() was already calling unset($this->services[$id]);.

Not sure I'll answer well your question, but have a look to the ProjectServiceContainer.

The protected $services is an empty array. has will however return true for the ->has('bar') call, because the getBarService() method exists. ->get('bar') will normally lazily create the service by calling the method, creating the instance and assign it to the service id. (However, it seems this container is missing the service id assignation in the getBarService(). Have a look to your appDevDebugProjectContainer eventually)

So, actually, a non-initialized service may not have its id in $this->services, thus isset($this->services[$id]) won't be sufficient.

Undertand, we should deal with the possible method mapping, right? I thought you meant adding array_key_exists here.

ro0NL · 2016-08-19T19:49:33Z

src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

@@ -839,6 +840,7 @@ public function __construct()

        $code .= "\n        \$this->services = array();\n";
        $code .= $this->addMethodMap();


I think we forgot $code .= $this->addPrivateServices(); here (see addConstructor), should we add it?

ro0NL · 2016-08-19T20:02:07Z

@nicolas-grekas @ogizanagi i think i got it pretty much covered. This day was crazy :-) My last commit seemed to be the missing piece.

However i cant really explain the failing 7.1 tests, it seems to crash and a bunch of unrelated errors. fabbot.io also wants me to break tests ;-)

And updated description.

ogizanagi · 2016-08-20T15:32:36Z

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

@@ -545,6 +545,9 @@ public function compile()
            if (!$definition->isPublic()) {
                $this->privates[$id] = true;
            }
+            if ($definition->isSynthetic()) {
+                $this->synthetics[$id] = true;


I know it was introduced this way for the $privates array, but to me, it doesn't make entirely sense.
Why not simply keeping an array of synthetics services' ids instead of this map ?

Even if it won't happen, that's misleading, as we use isset($this->privates/synthetics[$idToCheck]) later in the codebase, which returns true even if the value in the map is false.

A simple in_array($idToCheck, $this->privates/synthetics) would make more sense.

Agree. I was also thinking $this->serviceTypes something to track&check things in 1 go.

$this->serviceTypes[$id] & self::IS_PRIVATE

@hhamon: Could you, if you can recall, tell us if there is any reason it was implemented this way in #19146 ?

Maybe it was supposed to hold the generated id instead of a boolean later ? (Thus, it still won't make sense for synthetics service to behaves the same).

Maybe for performances reasons ? I doubt it'll be called often enough to have a real impact, but it probably deserves profiling it with blackfire on a real application.

Easy/fast isset i guess.

ro0NL · 2016-08-23T08:30:14Z

Still considering we should deprecate (ie. track) previously declared services, perhaps we're making it unnecessarily complex.

Like i explained in #19683 (comment); from the container pov this really doesnt matter too much. Although i can imagine we dont want to allow for this specific behavior on our implementation.

Long story short; im not sure we should keep tracking stuff like this and maybe think more compiler passes. Ie. #19683 rules out the $this->privates property by design. I like that :)

nicolas-grekas · 2016-08-23T08:34:27Z

src/Symfony/Component/DependencyInjection/Container.php

@@ -66,6 +66,7 @@ class Container implements ResettableContainerInterface
    protected $services = array();
    protected $methodMap = array();
    protected $privates = array();
+    protected $synthetics = array();


we don't need this is we don't put synthetic services in $this->methodMap, should allow simplifying many things

Are you sure? https://github.com/symfony/symfony/blob/master/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php#L883

But yeah, maybe we shouldnt. Relates to #19690 (comment)

edit: We lose service id's that way

Method for synthetic services throw exceptions, which means they are functionally useless. So for me it's safe to remove them from methodMap yes.

True. They need to be included in getServiceIds though.. however falling back to method_exists/get_class_method at runtime will cover.

Which is only needed to cover synthetics.. ie. having it al in methodMap would avoid such calls (as we can lazy load the map once).

they don't need to be listed in getServiceIds when they are not yet defined to me

Could be, perhaps ideally we should just deprecate this method and dont care :) it can be
interpreted in different ways.

From the component pov i think it should include all possible validly id's (not depending on state).

ro0NL · 2016-08-23T12:50:08Z

Different approach; only disallow overwriting initialized services, including privates.

nicolas-grekas · 2016-09-06T10:23:45Z

src/Symfony/Component/DependencyInjection/Container.php

        if (null === $service) {
-            unset($this->services[$id]);
+            @trigger_error(sprintf('Unsetting a service ("%s") is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);


the previous message was fine, and I'd keep the existing style for new messages too (the "%s" service/alias vs a service (%s))

ro0NL · 2016-09-07T07:39:47Z

I can work on it this week(end).

nicolas-grekas

Looks still good to me, and I think there should be a next step (later PR): since laziness is going to be a first class citizen, I think we might allow resetting a service again using set('foo', null) only and only if the foo service is lazy, either in the current way, or in the new ways (closure-proxy, etc). This would basically generalize #19203 (which could then be moved back to a simple set($name, null).

nicolas-grekas · 2017-01-02T19:19:19Z

src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php

+
+    /**
+     * @group legacy
+     * @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered


to be replaced by @expectedDeprecation instead

nicolas-grekas · 2017-01-02T19:20:42Z

src/Symfony/Component/DependencyInjection/Container.php

        if (isset($this->privates[$id])) {
            if (null === $service) {
                @trigger_error(sprintf('Unsetting the "%s" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
                unset($this->privates[$id]);
            } else {
                @trigger_error(sprintf('Setting the "%s" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0. A new public service will be created instead.', $id), E_USER_DEPRECATED);
            }
+        } elseif (null === $service) {
+            @trigger_error(sprintf('Unsetting the "%s" service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);


nicolas-grekas · 2017-01-02T19:27:13Z

src/Symfony/Component/DependencyInjection/Container.php

+            if (isset($this->services[$this->aliases[$id]])) {
+                @trigger_error(sprintf('Replacing the initialized "%s" service alias is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
+            } else {
+                @trigger_error(sprintf('Replacing the "%s" service alias is deprecated since Symfony 3.2 and will set the aliased service instead in Symfony 4.0.', $id), E_USER_DEPRECATED);


this message is strange: how is one supposed to patch its code to not get it anymore?

Not sure.. the behavior is strange also :). Imo. you cant override an alias, per definition.

So the path would be; dont set the alias?

ro0NL · 2017-01-02T20:42:06Z

Sounds good.

set('foo', null) only and only if the foo service is lazy,

I think we need a reset-able container for this, ie. reset($service = null) maybe? Not sure about magic null + set() here..

nicolas-grekas · 2017-01-08T15:40:09Z

src/Symfony/Component/DependencyInjection/Container.php

        }
+
+        if (isset($this->aliases[$id])) {
+            if (isset($this->services[$this->aliases[$id]])) {


This "if" should be removed, we shouldn't add more conditionals here.

nicolas-grekas · 2017-01-08T15:41:35Z

src/Symfony/Component/DependencyInjection/Container.php

+
+        if (isset($this->services[$id])) {
+            @trigger_error(sprintf('Replacing the initialized "%s" service is deprecated since Symfony 3.3 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
+        }


there is a missing case here, which is in fact what's described in #19192: initialized or not, we should deprecate setting a pre-defined non-synthetic services. This means we should check methodsMap and if there is a match, trigger the deprecation.

ro0NL · 2017-01-11T21:04:24Z

Updated. @nicolas-grekas can you have one more look at tests.. i had to do some changes.

edit: yeah.. how we detect if a service is synthetic?

ro0NL · 2017-01-11T21:07:01Z

src/Symfony/Component/DependencyInjection/Container.php

        if (isset($this->privates[$id])) {
            if (null === $service) {
                @trigger_error(sprintf('Unsetting the "%s" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
                unset($this->privates[$id]);
            } else {
                @trigger_error(sprintf('Setting the "%s" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0. A new public service will be created instead.', $id), E_USER_DEPRECATED);
            }
+        } elseif (null === $service) {
+            @trigger_error(sprintf('Unsetting the "%s" service is deprecated since Symfony 3.3 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
+            unset($this->services[$id], $this->aliases[$id]);


This should probably unset methodMap[$id] as well..

nicolas-grekas · 2017-01-11T21:34:33Z

src/Symfony/Component/DependencyInjection/Container.php

+            @trigger_error(sprintf('Replacing the initialized "%s" service is deprecated since Symfony 3.3 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
+        }
+
+        if (isset($this->methodMap[$id])) {


elseif with previous if so we're sure to trigger only one notice ?

set('kernel', $x) breaks a lot of tests now.. i was thinking to additionally track synthetic ids, or maybe simpler; dump a special method name func synthetic_getFooService()?

or remove synthetic services from methodMap? they don't need to be there to me (neither to we have do generate any method for them), wdyt?

see #21244 also

nicolas-grekas · 2017-01-11T21:35:34Z

src/Symfony/Component/DependencyInjection/Container.php

        }
+
+        if (isset($this->aliases[$id])) {
+            @trigger_error(sprintf('Replacing the "%s" service alias is deprecated since Symfony 3.3 and will set the aliased service instead in Symfony 4.0.', $id), E_USER_DEPRECATED);


I'm sure I'm missing something here :)
What will be the code/behavior you'd expect on 4.0? Not an exception?

$id = $this->aliases[$id] and forward :) i think it's ok.

return $this->set($this->aliases[$id], $service); actually.

OK. I'd be stricter here, and plan to throw also. No need for any special case, from the outside POV, alias or not, redefining something should throw.

I was also leaning that way.. makes sense. Lets do #21244 first, ill finish this one the weekend.

nicolas-grekas · 2017-01-11T21:48:59Z

ContainerBuilder also needs an update, isn't it?

…ted methods (nicolas-grekas) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Remove synthetic services from methodMap + generated methods | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - For synthetic services, the generated methods are just dead code to fill opcache ;) And having them in "methodMap" prevents using the property for checking if a service comes from a (non-synthetic) definition or not. This prepares some changes that we'd like to do in 4.0, see #19668. Commits ------- c1e1e99 [DI] Remove synthetic services from methodMap + generated methods

ro0NL · 2017-01-15T17:23:23Z

Rebased. Looking at ContainerBuilder now.. not sure what to do :)

ContainerBuilder::get() throws a runtime exception if it tries to create synthetic service from definition, we bypass Container::get() due invalid behavior mode. Not sure we should throw a service not found here as well?

ContainerBuilder::has() should probably check for the existence of an aliased id (like Container::has() now does). Ie. $id = (string) $this->aliasDefinitions[$id].

ContainerBuilder::set() again not sure :) do we need this feature anyway? Otherwise i propose to deprecate it for existing definitions (including aliases).

nicolas-grekas · 2017-02-08T10:03:25Z

I think this one should be closed in favor of #21533: setting no null is a legitimate (while risky I agree) operation while bootstrapping. 👎 here formally :)

ro0NL · 2017-02-08T10:22:54Z

Figured it out :)... this is not gonna happen.

This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Deprecate (un)setting pre-defined services | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | no | Fixed tickets | #19192 | License | MIT | Doc PR | - This PR is the subset of #19668 that fixes #19192: it deprecates (un)setting pre-defined services. This opens the path to some optimizations in the dumped container in 4.0. Commits ------- fdb2140 [DI] Deprecate (un)setting pre-defined services

carsonbot added Status: Needs Review Deprecation labels Aug 19, 2016

stof reviewed Aug 19, 2016
View reviewed changes

ro0NL reviewed Aug 19, 2016
View reviewed changes

ro0NL changed the title ~~[DI] Deprecate Container::set for nulls~~ [DI] Deprecate Container::set/get for nulls Aug 19, 2016

ogizanagi reviewed Aug 19, 2016
View reviewed changes

ro0NL changed the title ~~[DI] Deprecate Container::set/get for nulls~~ [DI] Deprecate Container::set for nulls Aug 19, 2016

ro0NL changed the title ~~[DI] Deprecate Container::set for nulls~~ [DI] Deprecate Container::set for nulls and existing services Aug 19, 2016

ro0NL reviewed Aug 19, 2016
View reviewed changes

ogizanagi reviewed Aug 20, 2016
View reviewed changes

nicolas-grekas reviewed Aug 23, 2016
View reviewed changes

ro0NL changed the title ~~[DI] Deprecate Container::set for nulls and existing services~~ [WIP][DI] Deprecate Container::set for nulls and existing services Aug 23, 2016

nicolas-grekas reviewed Sep 6, 2016
View reviewed changes

javiereguiluz added this to the 3.3 milestone Nov 7, 2016

nicolas-grekas reviewed Jan 2, 2017

View reviewed changes

nicolas-grekas reviewed Jan 8, 2017

View reviewed changes

ro0NL commented Jan 11, 2017

View reviewed changes

nicolas-grekas reviewed Jan 11, 2017

View reviewed changes

nicolas-grekas mentioned this pull request Jan 12, 2017

[DI] Remove synthetic services from methodMap + generated methods #21244

Merged

ro0NL mentioned this pull request Jan 15, 2017

[PhpUnitBridge] Assert missing deprecations #21296

Closed

[DI] Deprecate Container::set for nulls, initialized and alias services

e27a04b

This was referenced Feb 4, 2017

[DI] Deprecate (un)setting pre-defined services #21533

Merged

[FrameworkBundle] Wire ArrayCache for annotation reader at bootstrap #21556

Merged

ro0NL closed this Feb 8, 2017

ro0NL deleted the di/container-set branch February 8, 2017 10:22

nicolas-grekas moved this from In Progress to Done in Laziness in core Feb 8, 2017

ro0NL mentioned this pull request Feb 14, 2017

[DI] Deprecate Container::isFrozen and introduce isCompiled #19673

Merged

nicolas-grekas modified the milestones: 3.x, 3.3 Mar 24, 2017

		@@ -839,6 +840,7 @@ public function __construct()

		$code .= "\n \$this->services = array();\n";
		$code .= $this->addMethodMap();

[DI] Deprecate Container::set for nulls, initialized and alias services #19668

[DI] Deprecate Container::set for nulls, initialized and alias services #19668

Conversation

ro0NL commented Aug 19, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL Aug 19, 2016 • edited

Choose a reason for hiding this comment

sstok commented Aug 19, 2016

ro0NL Aug 19, 2016 • edited

Choose a reason for hiding this comment

ro0NL commented Aug 19, 2016 • edited

ro0NL commented Aug 19, 2016 • edited

stof commented Aug 19, 2016

ro0NL commented Aug 19, 2016

ogizanagi commented Aug 19, 2016 • edited

ro0NL commented Aug 19, 2016 • edited

ogizanagi commented Aug 19, 2016 • edited

ro0NL commented Aug 19, 2016

ogizanagi Aug 19, 2016 • edited

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

ro0NL commented Aug 19, 2016 • edited

ogizanagi Aug 20, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogizanagi Aug 20, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL commented Aug 23, 2016 • edited

Choose a reason for hiding this comment

ro0NL Aug 23, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL Aug 23, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL Aug 23, 2016 • edited

Choose a reason for hiding this comment

ro0NL commented Aug 23, 2016

Choose a reason for hiding this comment

ro0NL commented Sep 7, 2016

nicolas-grekas left a 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

ro0NL commented Jan 2, 2017 • edited

Choose a reason for hiding this comment

nicolas-grekas Jan 8, 2017 • edited

Choose a reason for hiding this comment

ro0NL commented Jan 11, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL Jan 11, 2017 • edited

Choose a reason for hiding this comment

nicolas-grekas Jan 11, 2017 • edited

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

ro0NL Jan 11, 2017 • edited

Choose a reason for hiding this comment

ro0NL Jan 11, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas commented Jan 11, 2017

ro0NL commented Jan 15, 2017 • edited

nicolas-grekas commented Feb 8, 2017 • edited

ro0NL commented Feb 8, 2017

ro0NL commented Aug 19, 2016 •

edited

ro0NL Aug 19, 2016 •

edited

ro0NL Aug 19, 2016 •

edited

ro0NL commented Aug 19, 2016 •

edited

ro0NL commented Aug 19, 2016 •

edited

ogizanagi commented Aug 19, 2016 •

edited

ro0NL commented Aug 19, 2016 •

edited

ogizanagi commented Aug 19, 2016 •

edited

ogizanagi Aug 19, 2016 •

edited

ro0NL commented Aug 19, 2016 •

edited

ogizanagi Aug 20, 2016 •

edited

ogizanagi Aug 20, 2016 •

edited

ro0NL commented Aug 23, 2016 •

edited

ro0NL Aug 23, 2016 •

edited

ro0NL Aug 23, 2016 •

edited

ro0NL Aug 23, 2016 •

edited

ro0NL commented Jan 2, 2017 •

edited

nicolas-grekas Jan 8, 2017 •

edited

ro0NL commented Jan 11, 2017 •

edited

ro0NL Jan 11, 2017 •

edited

nicolas-grekas Jan 11, 2017 •

edited

ro0NL Jan 11, 2017 •

edited

ro0NL Jan 11, 2017 •

edited

ro0NL commented Jan 15, 2017 •

edited

nicolas-grekas commented Feb 8, 2017 •

edited