Skip to content
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

Lazy-loading of command dependencies #7

Closed
f3l1x opened this issue Jul 15, 2017 · 15 comments

Comments

2 participants
@f3l1x
Copy link
Member

commented Jul 15, 2017

No description provided.

@f3l1x f3l1x self-assigned this Jul 15, 2017

@enumag

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2017

I don't think this is necessary at this point. https://symfony.com/blog/new-in-symfony-3-4-lazy-commands

@f3l1x

This comment has been minimized.

Copy link
Member Author

commented Jul 15, 2017

@enumag Thanks for the link. That looks cool. I have to figure only thing out - CommandLoader.

@enumag

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2017

@f3l1x That should be easy, right? The default ContainerCommandLoader from symfony is for PSR-11 container. So you either implement your own for Nette DI or use the default one with Arachne/ContainerAdapter.

@f3l1x

This comment has been minimized.

Copy link
Member Author

commented Jul 15, 2017

@enumag It looks easy, you're right.

PSR-11 is already in my roadmap -https://github.com/contributte/psr11-container-interface

@enumag

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2017

@f3l1x Are you planning to add PSR-11 intertface to Nette/DI container directly or just make an adapter like I did?

@f3l1x

This comment has been minimized.

Copy link
Member Author

commented Jul 15, 2017

PSR-11 into Nette/DI is little bit complicated, I have to discuss it with @dg. For now I would like to create just an adapter.

@enumag

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2017

@f3l1x In my opinion it's useless to create another PSR-11 adapter package since I already have one only to deprecate it a few months later when we figure out a way to add it to Nette DIC directly.

But if you still want to make your own and force me to deprecate my package and switch dependecies, I can live with it. If you find anything useful in my package feel free to copy-paste it.

@f3l1x

This comment has been minimized.

Copy link
Member Author

commented Jul 15, 2017

@enumag I'm very glad that you said that. I think there are many useful packages. I've invited you to our private slack (contributte, planette, etc..). Please contact me there ;-)

f3l1x added a commit that referenced this issue Nov 9, 2017

@f3l1x

This comment has been minimized.

Copy link
Member Author

commented Nov 9, 2017

This is ready & hot.

If you use symfony/console: <3.4, there's no change for you.

If you use symfony/console: >=3.4, you have to enable lazy-loading.

console:
    lazy: true

That's all. 🎉

@f3l1x f3l1x closed this Nov 9, 2017

@f3l1x f3l1x added this to the v0.2 milestone Nov 9, 2017

@enumag

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2017

@f3l1x Why is it not enabled by default?

@f3l1x

This comment has been minimized.

Copy link
Member Author

commented Nov 9, 2017

@enumag Because of versions, this extension support symfony/console 3.2,3.3 and the new one 3.4. It will be default in next releases.

Does it make sence?

@enumag

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2017

It does although I'd probably do it differently: do you have 3.4+, great you have it lazy... oh legacy 3.2/3.3? no laziness for you...

I'd probably not even bother with implementing the option. :-D

@f3l1x

This comment has been minimized.

Copy link
Member Author

commented Nov 9, 2017

I'm in, how do you want to detect it?

@enumag

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2017

How about simply testing existence of the CommandLoaderInterface?

@f3l1x

This comment has been minimized.

Copy link
Member Author

commented Nov 12, 2017

Well, I like my solution more. I don't like to depend a version check on class_exists function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.