Sylius / Sylius

Open Source eCommerce Framework on Symfony
https://sylius.com
MIT License
7.91k stars 2.09k forks source link

[RFC] Introducing components #549

Closed pjedrzejewski closed 10 years ago

pjedrzejewski commented 10 years ago

Hey folks! This is a big one, so please fasten your seatbelts.

I'd like to extract everything not symfony-specific into standalone components. I don't think I have to explain the idea, because it's exactly the same thing as with Symfony. Library + bundle, which is integration layer with the framework.

I'll start with 1 fun fact about this idea... I already did it, twice.

First, in quite early stages of development, when Sylius had much less features than now. I reverted this, because I had a feeling that the components were very, very small and simply not useful for anything else than Symfony2 bundles.

Second, it was not so long time ago, but before the reorganization of repositories on github. We had around ~20 (or even more) active bundles, so the change resulted in 40 repositories to maintain/fork/track issues and so on... it was a bit painful to manage, believe me.

With our current github structure and use of subtree split, it's super simple to do and manage. I created bash scripts for subtree splits and tagging releases, so it's just matter of adding new repos to the list.

Now, I want to do this third and last time. :)

Pros & Why...

Cons...

Problems to tackle during the change...

The extraction of components itself will be pretty simple, but there are some questions.

  1. Where should we put the doctrine mappings? Libraries or bundles?
  2. I'm on the edge with models. I'm pretty sure that we'll end up putting them in the components, but I'd like to hear your opinion. Models represent the business domain, so it does not feel entirely right to have them "so far away" from the application source code. (in the component) On the other hand, Sylius components would be slightly different (not in all, but most cases) from other php libraries, because they do not necessarily belong to low level (framework) like Finder or HttpKernel components from Symfony.
  3. With this reorganization, I have this idea of simplifying the namespaces. I'm not entirely convinced yet, but what do you think about these proposals?

Sylius\\Bundle\\FooBundle\\ Sylius\\Component\\Foo\\

or

Sylius\\FooBundle\\ Sylius\\Foo\\

or leaving the component.

Sylius\\FooBundle\\ Sylius\\Component\\Foo

Reason is simple, TaxationBundle already contains the "bundle" suffix, so there will be no conflicts / confusion. And it makes things shorter.

What do you think?

/cc @weaverryan (we talked several times about it, I hope you're proud I finally wrote this RFC :D)

pborreli commented 10 years ago

:heart_eyes_cat:

Richtermeister commented 10 years ago

:metal: This seems to be approach taken by the Vespolina as well, which begs the question whether there could be some more synergy between the projects.

But yeah, totally for this & happy to help.

cryptocompress commented 10 years ago

Sounds reasonable but there should be two/three concrete projects or this may end up ungainly. => know how easy it will be to use

pjedrzejewski commented 10 years ago

I think it would be good idea to ask folks from the Laravel community or even ZF2, if they would be interested in reusing Sylius with these frameworks. :)

arnolanglade commented 10 years ago

I think it is good idea, Sylius can be use everywhere, the community can grown up...

I am for : Sylius\\FooBundle\\ Sylius\\Component\\Foo

I wait for ResourceBundle refactoring RFC, I can help too. I have some ideas about global configuration (we need to keep my previous PR closed and not merged). I can open a topics to share my ideas if you want, Pawel.

adrienbrault commented 10 years ago

About namespaces:

Sylius\\Foo Sylius\\Bundle\\FooBundle

stloyd commented 10 years ago

About naming (IIRC we talked already few times about this @pjedrzejewski), if in any way I would try to convice you to clean way like (somehow) used by Symfony, i.e.:

Yes, I know that most of those points add some complications, but in case of such changes, we should have a look at already proven OS projects, and Symfony with this approach fits very well IMO.

ps. Pawel, not so much trolling =) I will be kind for you today =)

Richtermeister commented 10 years ago

@stloyd :+1:

pjedrzejewski commented 10 years ago

I agree with all your points, but I see a lot of issues/questions regarding the 2nd idea.

To me, the "bridge" which connects stuff all together, will be "Core" component. (extracted from CoreBundle) I hardly see what could we put in such bridge, for example, core Order connects almost all components together, core Product connects many different components, like shipping, taxation, taxonomies, inventory ... What would go to such bridge? I see an use case for a bridge between OrderBundle and PromotionsBundle, that's pretty simple, we could put the actions there.

Generally I'm :+1: for the Bridges idea, but we should be very careful how we execute this concept. Just imagine face of a developer who hears that he has to use the TaxRate model from Taxation component, configured in TaxationBundle, but extend TaxRate entity from AddressingTaxationBridge to have tax zones support... Things get pretty complex here.

marcospassos commented 10 years ago
Sylius\Bundle\Foo\
Sylius\Component\Foo\

Thus, I go with (otherwise we will end up mixing bundles and libraries):

Sylius\Bundle\FooBundle\
Sylius\Component\Foo\
jjanvier commented 10 years ago

My 2 cents

Sylius\Bundle\FooBundle\
Sylius\Component\Foo\

For the same reasons as described by @stloyd. Just a little bit afraid about this Bridge thing :

On the other hand, CoreBundle (or CoreComponent) is too big...

cordoval commented 10 years ago

Nice! About V it may have not been presented as a complete solution ready to install like sylius but trust me, v components are running in production and probably more in a decoupled way (disagreeing with @marcospassos about the not being ready status, it is very subjective. I was told not to write a book but seriously then the next day there was a showcast of sites built with sylius displaying on sylius.org, the understanding of this is not to fuss but to just convey that devs can do pretty good things when they adapt code to their needs if the components allow it). Sometimes I believe ecommerce is so flexible that many people, even more than the ones running S/S, end up with a fully custom massive ecommerce and they just get some inspiration from components. I believe that is the power of components. Those that have seen Aura project know what i am talking about. I see with good eyes that Sylius components become more like that. Cheers!

cordoval commented 10 years ago

I agree with @adrienbrault on the folder things

molchanoviv commented 10 years ago

I totally agree with @stloyd

HellPat commented 10 years ago

Cool. I'd like to use components in Laravel.

drgomesp commented 10 years ago

Finally...

pjedrzejewski commented 10 years ago

@drgomesp I count this as +1. :)

BenoitLeveque commented 10 years ago

this seems to be a good idea

drgomesp commented 10 years ago

@pjedrzejewski To be honest with you, I quit using Sylius on a huge project of my company – where I am the Lead Developer and suggested Sylius at the beginning – mainly because of the high coupling it has with Symfony. I think this is the first step to create a better framework, but honestly I think a lot of changes need some sort of review.

One of the most important things an open-source project needs is a well-written and up-to-date documentation – Sylius clearly misses that. Besides, the project needs more consistence on the domain design – that including the services and, of course, the entity relationships. One example of what I'm saying is the relationship between Order and Cart, that I already talked about in some issue around here.

Consider this as my contribution and as my statement to be 100% willing to help the project to grow. You can count on me. And some other link to contribute with the other ones: http://danielribeiro.org/yes-you-can-have-low-coupling-in-a-symfony-standard-edition-application/

And yes, this is a +1. :)

weaverryan commented 10 years ago

Obviously, you know that I would like to see this :). But:

1) Like with a normal project, it may be better to do it little by little. If you have lots of time, you could do it all at once. But to keep things usable, you could do it one piece at a time. That would also give you time to learn how this process feels for some components and use those lessons on the future components.

2) What's the dialog (if any) between Vespolina and Sylius? It's not that you would need to work together (look at Zf2 and Sf2 for example), but at least talking with those guys may bring about some opportunities. Also, you may have already done this - I don't know :).

3) If you're hoping that other projects (like Drupal, ZF2 people) will someday use some Sylius components, then I think it's imperative to find those people and talk to them now. We may think we know what types of solutions they need, but really only they know. By talking with them, it will help shape the direction towards what's really needed and not what we think is needed. It may also mean some early participation because we've started a dialog really early.

So, what the plan with this RFC from here?

jjanvier commented 10 years ago

1) @weaverryan actually, that's almost already done ! See https://github.com/Sylius/Sylius/issues/586. Of course this needs to be polished, but it was quite simple to split the business logic and Symfony related stuff.

2) and 3) @pjedrzejewski will surely answer better than me :)

pjedrzejewski commented 10 years ago

@weaverryan Thanks for joining us Ryan! :)

  1. You underestimate us... 8 days, 125 commits and we have a green build with components. :) https://github.com/Sylius/Sylius/pull/595 I'm going to tag v0.6.0 and then merge it + setup syncing for new repos, packagist. I hope everything to be completed this weekend.
  2. We talked few times and while I have an impression that both sides are open for collaboration (just look at Sylius vendors), nothing happened. Probably it's my "fault", because I have quite different vision for a lot of stuff, different inspirations and so on. I believe at some point we'll find an area which can be shared/done together.
  3. Definitely agree, I'm planning to reach out Drupal, ZF2 and Laravel at first. I do not have too many contacts in these communities but hey, it can't be that hard. :)
cordoval commented 10 years ago

@weaverryan cc/ @iampersistent @inspiran cc'eing just in case they miss it.

Thinking constructively, i think there could be a way to decouple components when things get more mature in a way that allows for other blocks to be plugged in from any side, adapters, common interfaces maybe. It would be a nice feat to have a common core of interfaces for instance for common tasks.

Another way to get our hopes up is if we can invite contributors from either side to contribute to both. I think we should not be too married to one or either project and see these as tools.

2 cents :baby:

iampersistent commented 10 years ago

I am not speaking for the Vespolina project, but my expressing own personal opinion. Personally, I think its ridiculous to have 2 different projects doing essentially the same thing. In some ways the approaches taken by V and Sylius are quite similar. Until now, the biggest difference was the change in focus with V to be more of a library than a SF2 specific solution. I had made that decision and looking back, it was probably the right decision at the wrong time.

My goal from the beginning was to have a single ecommerce library that could be shared and save everyone a lot of time. That is still what I want to see. I don't know how V and S wants to settle the differences, but I personally have no desire to write redundant code.

alexsegura commented 10 years ago

Hello everyone,

At the company I'm working for we are seriously considering Sylius, but we are not using Symfony. So the goal is clearly to add e-commerce features to an existing webapp. We are also really interested by the subscriptions bundle (#385)

What is the status of the components split ? Is it abandoned ? I've seen the components branch, but it hasn't moved since 3 months.

How is it possible to help ? Just sending pull requests to the components branch ?

jjanvier commented 10 years ago

@alexsegura the component split is not abandoned. But is has been delayed (edit: not delayed, but rescheduled ^^) by @pjedrzejewski. Only him will be able to give you an estimated date for this.

@Sylius I'm afraid we'll have to drop the components branch to start again from scratch... Master and components branches have diverged so much that rebasing and merging will surely be very difficult and a waste of time. Anyway, we can at least try :)

elliot commented 10 years ago

I was thinking the same. Happy to volunteer to help with splitting the code base again, last time was a great learning experience for finding the cooler refactoring tools in PHPStorm. Just say the word.

drgomesp commented 10 years ago

@pjedrzejewski how is the progress on this one?

I'm thinking of start working on this. What would be a good starting point?

alexsegura commented 10 years ago

Thanks @jjanvier for responding

I understand this split is something huge, maybe the Pull Requests should stop being merged until the split is finished ?

BTW can't we automate the split process ? If it's "just" a matter of changing namespaces and moving folders, maybe a bash script with sed -i could help ?

jjanvier commented 10 years ago

@alexsegura the split process is not so complicated. If everything is well prepared (ie: if specs use typehint instead of these ugly comments), this can be done by 1 person in 1 day with a good refactoring tool (I used PhpStorm) IMO. This is just an assembly-line work. And it's better than a script as you see what you are doing.

I higly discourage someone to do this again until @pjedrzejewski answers ^^ He did not merge this at the time because he had good reasons (at least, that's what I think). It's only a matter of time. This split will come for sure.

About stopping merging new PRs... Yeah... Agree... But sadly that's not what has been chosen.

elliot commented 10 years ago

I just finished converting all the docblock notation in the specs over to typehints in #1092 (minus ResourceBundle for @aRn0D :wink:) so this time around it shouldn't be nearly as much work and the refactoring tools can do all the heavy lifting.

QuingKhaos commented 10 years ago

@pjedrzejewski Introducing components may be a big BC break if anyone uses models and stuff programmaticaly? May you tag before this a working version with correct dependencies in every composer.json?

winzou commented 10 years ago

This is anyway just a namespace renaming so that's not the worst BC we can do ;) Le 20 févr. 2014 18:42, "Patrik Karisch" notifications@github.com a écrit :

@pjedrzejewski https://github.com/pjedrzejewski Introducing components may be a big BC break if anyone uses models and stuff programmaticaly? May you tag before this a working version with correct dependencies in every composer.json?

Reply to this email directly or view it on GitHubhttps://github.com/Sylius/Sylius/issues/549#issuecomment-35596032 .

QuingKhaos commented 10 years ago

Yep, but it's a big (nearly every class(?)) BC break if you really on many stuff in your own bundles ^^

Btw. it would be cool to have a 0.x release with correct dependencies, which you can install. Unfortunately v0.7.0 is broken, since all sylius deps were left 1.0.*@dev.

elliot commented 10 years ago

Sylius is still pre-alpha, so there is going to be some breakage up until at least the first alpha.

That said, this change is isolated to renaming namespaces and has no functional impact. Are you using a decent IDE like PHPStorm? If you are then it will be a breeze to update.

QuingKhaos commented 10 years ago

I know :smiley_cat: But there are already 0.x versions, while not release early, release often? According to semver.org all 0.x versions are unstable versions (pre-alpha).

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

And I know there will be BC breaks, and they are not the big problem with IntelliJ, but I like also to have unstable versions to jump from one to another instead of relying on dev-master, which may break at any commit :bowtie: I know I'm annoying with this and sorry for that, but I like the idea of release early, release often :neckbeard:

Richtermeister commented 10 years ago

@elliot what @patkar is trying to say is that some of use have apps in production which extend the original models, so they will need manual patching, and it would be nice to leave a tag behind, just in case.. But yeah, not that concerned myself.

stloyd commented 10 years ago

@Sylius I think it's time for reintroduction of this idea, we are getting stuff in bundles that create some kind of crap like BuzzBundle (not yet that crappy, but much closer) with things like Calculators, PricingBundle, MoneyBundle.

Those parts are crap* not real bundles and we should never create such stuff as it creates more confuses & and problems in making them standalone than solving any real problems.

I will start splitting this one more time in next days if we agree we are still going with this idea, which is IMO must have to not lock ourselfs in problems like: man, why everything is in CoreBundle/ResourceBundle?!

* - I'm not talking about ideas per se, I'm talking about creating overkills like bundles instead of components - aka small re-usable, well designed micro-libraries.

QuingKhaos commented 10 years ago

:+1:

pjedrzejewski commented 10 years ago

I don't see how Pricing or Money bundle is an overkill, how would you separate them from other bundles otherwise? Put all in 1 bundle? They have quite a lot of configurations and form types or even doctrine mappings.

I'm all for starting this again, but let's do it the same way as we did with @jjanvier. Every bundle to component, then we can think about moving things around more.

jjanvier commented 10 years ago

@pjedrzejewski Is it a real "go" ? I mean, if we split bundles and components, are we sure this is going to be merged quite quickly ?

pjedrzejewski commented 10 years ago

Real GO! Do you have some scripts to do this? (from the last time?)

jjanvier commented 10 years ago

No script. Only the "phpstorm way" I've shared on a comment somewhere. But with the PR done by @elliot on the specs, we will go as fast as light. Expect a few PRs tonight ;)

pjedrzejewski commented 10 years ago

@jjanvier :+1:

jjanvier commented 10 years ago

@pjedrzejewski can we drop the components branch and create a new one ?

stloyd commented 10 years ago

@jjanvier Let's do this step by step for every bundle, first let's change phpspec, from comments into typehinting, what do you say?

arnolanglade commented 10 years ago

Did someone already do that ?

jjanvier commented 10 years ago

@stloyd AFAIK it's already done. Still some specs bundles with comments ?

stloyd commented 10 years ago

@jjanvier TBH didn't check :trollface: But I would not remove the old branch yet, we can use it as "history", yet we can't create and commit to branch in main repo, so it needs to be done by @pjedrzejewski and later via PRs by someone else.

jjanvier commented 10 years ago

@stloyd ok, let's keep it for history. Can we at least create a new branch to open PRs on ?