FriendsOfSymfony / friendsofsymfony.github.io

Description of what FriendsOfSymfony (aka FoS) is about.
http://friendsofsymfony.github.io
32 stars 4 forks source link

new FOSMessageBundle maintainer #54

Closed lsmith77 closed 9 years ago

lsmith77 commented 9 years ago

As per https://github.com/FriendsOfSymfony/FOSMessageBundle/issues/229 we need a new maintainer for FOSMessageBundle.

https://github.com/tgalopin is interested in stepping up. he is working on a promising refactoring, splitting the core pieces off, making them Symfony independent: https://github.com/tgalopin/FOSMessage

He has also created a Bundle using this lib: https://github.com/tgalopin/FOSMessageBundle

Note that this is still a work in progress and that the current state is still somewhat close to the original for some of the class names. But you can also see the decoupling from the Symfony Dispatcher and Container. Some code was added for the case when those Symfony dependencies are not around, maybe in the end they will go to another repository. Its not yet clear if the new library should end up in the FOS namespace, as this might associate it too closely with Symfony.

What I would propose: 1) make https://github.com/tgalopin official maintainer 2) move his new Bundle into a "develop" branch to more easily gather feedback from current users

willdurand commented 9 years ago

:+1:

We need new motivated people.

tgalopin commented 9 years ago

Thank @lsmith77,

Actually, while the two repositories you linked are the right ones in the sense that they port the framework-independant concepts out of the bundle, they don't represent properly the new library.

@ChristianRiesen and I discussed quite a lot about how the current concepts of the library are not well named and the library itself is not intuitive at all for new comers. As the new version has to break backward compatibility in some ways to improve, we thought it could be a better idea to rethink it from scratch. We did this and I'm currently working on the completely new library here:

https://github.com/tgalopin/message

In the same time, I know well the current bundle code as I ported it in the repositories you mentionned and I think I'm able to work on it. Here is my proposal:

lsmith77 commented 9 years ago

sounds good to me as well ..

ojhaujjwal commented 9 years ago

+1 from me as well

stof commented 9 years ago

sounds good to me. :+1:

I have a few remarks regarding the early draft of the library though:

tgalopin commented 9 years ago

@stof thanks for your feedback!

I agree with you for the private visibility, i'll change that.

For the second point, we provide a "god class" in order to ease the developer job. Perhaps should this god class be a shortcut for other services with less responsabilities. Something like MassagingService::startConversation() -> MessageSender::createConversation() & MessageSender::sendMessage(), etc. What do you think?

stof commented 9 years ago

@tgalopin Adding a higher level API to simplify the usage may be good. But we should first design the lower layer and see which places actually need to have an higher level API to simplify things. If the god class is just proxying calls, it does not provide much value. Using the dedicated service directly will not be harder.

and a single class doing everything is IMO harder to understand as you cannot understand (or describe) its goal easily. It also make it harder to implement new features in a BC way (remember that adding new methods in an interface is a BC break)

ojhaujjwal commented 9 years ago

I agree with @stof

tgalopin commented 9 years ago

I completely agree with you, but this was a debate I had with @ChristianRiesen and he prefered a single class for usage simplicity. I'll think a bit more about it.

If any of you have a specific idea in mind about what you would do for the global architecture of the library, please don't hesitate to propose it (perhaps in an issue on the repository: https://github.com/tgalopin/message/issues), I'd be happy to discuss it with you!

aitboudad commented 9 years ago

:+1:

tgalopin commented 9 years ago

As you proposed @stof, I splitted the Messaging Service into three smaller components (https://github.com/tgalopin/message/tree/master/src): the Repository (fetch messages / conversations / persons), the Sender (start conversations and send messages) and the Tagger (add / remove tags). I'll add new components as the library grow.

However, it brings a problem I discussed with Lukas and Christian: now, some services depends on others (Sender depends on Repository for instance). If I ask developpers to create objects the way I want, it requires boilerplate code just to setup the library components and if i provide a global Factory able to create the services, it's difficult to stay as flexible as I was with separated components.

So my question is: is there a proper way to manage services like I would in a service container without depending on a specific DIC implementation? Are there libraries that successfully solved this problem or does it require a PSR (like we talked with Lukas)?

stof commented 9 years ago

@tgalopin my proposal is to provide an optional factory building the necessary objects for standalone usage. Then the bundle could skip this factory and create the object directly to have more flexibility on the way to configure them in the Symfony DIC.

See https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/Form/Forms.php (and the related builder class) for an example of such pattern in Symfony (the Form component involves a bunch of objects to setup, and this simplifies it). FrameworkBundle does not use this class

tgalopin commented 9 years ago

I like this idea. I'll do that, thanks!

stof commented 9 years ago

@tgalopin in any case, you should start by building the library. Adding an helper class to build everything is something which should come at the end, not at the beginning IMO

tgalopin commented 9 years ago

I was going to do that yes. I'll do the library, then isolate components needing to be easy to instanciate and provide the factory for them.

dbu commented 9 years ago

i am late to the party but :+1:

tgalopin commented 9 years ago

Some issues are waiting for being closed / answered in the bundle. Is everyone fine with me concretely becoming the maintainer? The warning in the README probably scares people :) .

dbu commented 9 years ago

lets make this happen!

tgalopin commented 9 years ago

Ping @lsmith77, @dbu @willdurand, I still receive messages from people asking for the new library as the old one has the message on the README.

dbu commented 9 years ago

I just created https://github.com/FriendsOfSymfony/FOSMessage and added it to the FOSMessageBundle group and added @tgalopin and @ChristianRiesen to that group. Thanks a lot for contributing!

I propose that you start with a pull request from your current repository to the FOS one where we can discuss basic architecture worries. We should however quickly merge that, and open issues for non-critical things that people note in the code.

@tgalopin please also update the README of FOSMessageBundle to reflect the change in maintainership.