deprecated-packages / symplify

[DISCONTINUED] Check split packages in their own repositories :)
MIT License
621 stars 189 forks source link

[SymfonyEventDispatcher] deprecation in favor of contributte #168

Closed TomasVotruba closed 7 years ago

TomasVotruba commented 7 years ago

Hey Guys,

I've got a question for you. Since Symfony 3.3, there have been added new EventDispatcher features, that
dropped added value of SymplifyEventDispatcher for Symfony.

Only usable case remains Nette. There is similar package by @f3l1x : https://github.com/contributte/event-dispatcher

That does pretty much the same thing. Last few months I've realized I can invest to package that add more than just a few % of added value, instead of maintaining a 90% similar product.

1. So I would be happy to deprecate this package in favor of @f3l1x one. That means, it would be still available for years and current composer supported versions, just not developed.**

My question is - would you consider that a good way to continue or not? And why?

2. I asked @enumag on PM, and he would be ok, just missing ::class based event naming instead of string one.

$this->eventDispatcher(ApplicationRequestEvent::NAME); 
// alows to open event right away

instead of

$this->eventDispatcher(ApplicationEvents::ON_STARTUP);
// contributte, string only

For me, that would a detail, since on my own events I can use ::class regardless the package.

What do you think? About 1. and 2.

/cc @f3l1x @enumag @Lexinek @fprochazka

f3l1x commented 7 years ago

šŸ‘


I like this whole thing. Don't split energy to more projects, rather focus on one. If it could be contributte/event-dispatcher I couldn't be happier.


I'm not sure what do you mean with $this->eventDispatcher(ApplicationRequestEvent::NAME);. It's just a name?

TomasVotruba commented 7 years ago

I though you'd be happy!


I'm not sure what do you mean with $this->eventDispatcher(ApplicationRequestEvent::NAME);. It's just a name?

The point is not the constant, but the event class. You can click to the event class and see it's arguments and methods. I tried my gif skills :camera:

dispatcher

f3l1x commented 7 years ago

@TomasVotruba I get it now. It might be useful. I could add it to contributte/event-dispatcher too.

fprochazka commented 7 years ago

FYI I intent to drop the dispatcher integration from Kdyby/Events too, and probably gonna add suggest for contributte :)

f3l1x commented 7 years ago

@fprochazka That sounds great. Where does this intent came from? :)

Do you mean to drop Kdyby/Evens or just symfony events or only nette packages events?

fprochazka commented 7 years ago

@f3l1x https://github.com/Kdyby/Events/issues/97#issuecomment-301213842

f3l1x commented 7 years ago

@fprochazka I get it now. :) Thanks.

TomasVotruba commented 7 years ago

@fprochazka Another point for this. Thank you.

@Lexinek What do you think? I'd like to hear your opinion.

JanMikes commented 7 years ago

1) In this case, as a user i always want the newest version, with bug fixes, new features etc... it is always hard to deprecate package, for me it is sign of that i should not use it anymore For example i am using symplify/symbiotic-controller that depends on symplify/symfony-event-dispatcher. Will you update it and replace symplify/symfony-event-dispatcher implementation with the one from @f3l1x ?

If the answer is YES, i am happy and i will approve this instantly for you. If the answer is NO, what can i, as user, do with that deprecation warning in composer install/update? I want to use a package that depends on deprecated package and i feel like i am doomend (i would never intentionally want to use deprecated package).

2) Definitely agree, i am using events this way and it is really addictive!

Overall i think this is good way to go, focus on more important things that will bring more added value to community and developers, instead of bringing the wood into forest (maintaining package solving problem that is already solved by other maintained package)

TomasVotruba commented 7 years ago

Thank you so much for your complete and easy to read answer.

  1. I say YES to this.

  2. In that case, please create an issue at conttribute package and let talk go on there.

Bringing wood to the forrest exactly describes my feeling. Nice one.

TomasVotruba commented 7 years ago

Resolved via https://github.com/Symplify/Symplify/pull/170

Thank you all for your words, ideas and opinions. It's much easier for me to decide upon many united voices.

TomasVotruba commented 7 years ago

It's all yours now @f3l1x Enjoy :heart:

image

TomasVotruba commented 7 years ago

@f3l1x Bit late, but I've just noticed my app stopped working after switching to your bundle.

To me while to figure out only contributte interface are automitically added, not Symfony\Component\EventDispatcher\EventSubscriberInterface. Not sure why, since it is empty.

This is a huge BC break, that would force all people using Symplify extension to change all their subscribers. Would you be open to use original interface?

/cc @Lexinek @enumag

f3l1x commented 7 years ago

Sure, I will change it. :)

TomasVotruba commented 7 years ago

@f3l1x That would be great :cake:

I'm sorry if my tone was too hursh, I had weird and tough night yesterday.

JanMikes commented 7 years ago

I definitely agree.

Btw i had similar bug in my application once and spent 2 hours finding out that in my use statement there is other EventSubscriberInterface than the one from Symfony :-)

TomasVotruba commented 7 years ago

Hehe, that must have hurt. Which one? :D

We might start some PHP Trollo package!

JanMikes commented 7 years ago

In my case it was JMS\Serializer\EventDispatcher\EventDispatcherInterface TBH i dont know why they have their own interface for this (it is empty, probably same use case as in this repository). I was writing class ... extends Dispatcher PHPStorm autocompleted for me the JMS namespace was on the first place and without checking i pressed enter and then i was doomed. It was crazy like hell, wtf factor 9/10, i almost jumped out of the window, i was like "WTF this subscriber works and this just does not" :-)

Imho best practice is to support only symfony event dispatcher interface, as it leads to terrible use cases such as i described, but it would cause BC Break to this (contributte) package.

enumag commented 7 years ago

@Lexinek Yeah I hate PHPStorm for suggesting the JMS Dispatcher. Is there some way to prevent PHPStorm from doing so?

JanMikes commented 7 years ago

@enumag i did not find any way to do it yet :-/

TomasVotruba commented 7 years ago

@enumag @Lexinek Could you try removing it?

enumag commented 7 years ago

@TomasVotruba No, we need the bundle where it is.

TomasVotruba commented 7 years ago

@enumag So - not the preties solution, but could work - maybe https://pehapkari.cz/blog/2017/01/20/jak-snadno-a-rychle-upravovat-soubory-ve-vendoru/

enumag commented 7 years ago

@TomasVotruba The bundle uses the interace internally so the change would be too heavy. This is not a good way to fix an IDE issue.

TomasVotruba commented 7 years ago

@enumag Too bad. One of few first things I learnt in Symfony world, was that JMS Bundles are better to skip and manage by own code.

enumag commented 7 years ago

Possibly. I didn't personally add it to the codebase so I'll investigate why it's there. It's probably for FOSRestBundle though and I don't know any other good bundle for rest API. Can you recommend any?

TomasVotruba commented 7 years ago

I think few guys might know good REST bundles or approach in Symfony /cc @fprochazka @vasekpurchart @mhujer

fprochazka commented 7 years ago

We've been using friendsofsymfony/rest-bundle + jms/serializer-bundle. And apart from often useless error messages from jms/serializer-bundle, I quite liked the setup. I didn't have to set it up tho, @VasekPurchart did it and I've just been using it, so I haven't really had a chance to hit the juicy stuff everyne is hating on.

VasekPurchart commented 7 years ago

Yes, our setup relied on JMS, which I agree is quite a pain to use sometimes, but I think this was caused mainly by the fact that the author was not actively developing them. We were using them because of their broad capabilities and quite good extension options and we had some custom made infrastructure set up on it :)

But anyway I didn't find any better combination of bundles to serve my purposes, so I wanted to implement my own, but of course, there was no time, so we were stuck with this. If my next project is a Symfony API though, I'll try to create it :)