solidusio / solidus

🛒 Solidus, the open-source eCommerce framework for industry trailblazers.
https://solidus.io
Other
5.02k stars 1.29k forks source link

RFC: Deprecate Spree namespace #3065

Closed dustineichler closed 5 years ago

dustineichler commented 5 years ago

I'd like to see Solidus deprecate the Spree namespace. It's time we move away from being a "fork" of Spree. Obviously this would be a big lift, but necessary while comparisons between Solidus and Spree are no longer 1:1 and haven't been for a while.

Questions, comments and concerns below.

jacobherrington commented 5 years ago

This is definitely worth talking about, we've moved far enough from the original Spree project that it's probably a good idea to try making that comparison a little easier for potential stakeholders.

I also know that there is the potential for causing some serious harm to extensions, etc.

jarednorman commented 5 years ago

Yeah, I'm personally pretty indifferent on this one (though it would be nice,) but if we do it, we definitely have to go with whatever approach causes the least problems.

dustineichler commented 5 years ago

@jarednorman agreed. i've looked into this. i see two issues: updating namespace and db migration e.g. - solidus_.

dhonig commented 5 years ago

Obvious issue with the plugins and echosystem as well.

On Thu, Feb 14, 2019 at 1:41 PM Dustin notifications@github.com wrote:

@jarednorman https://github.com/jarednorman agreed. i've looked into this. i see two issues: updating namespace and db migration e.g. - solidus_.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/solidusio/solidus/issues/3065#issuecomment-463742334, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOZrzBP-1UiBHM9J3G5PW_flsPrQI6fks5vNa3WgaJpZM4aa6aP .

patleb commented 5 years ago

@jacobherrington, project namespace is an implementation detail, I don't see how this could affect the stakeholders matter (the project is already marketed as Solidus).

I don't mean to be harsh, but I really don't see any advantages to change namespacing just for the sake of it: I would say that it could potentially do more harm than good and it doesn't clarify anything (quite the contrary, you know from where the code comes from --> Spree).

As someone who works with both projects (Spree and Solidus), I can tell that there is more similar code than completely different one and both are using almost identical data modeling and ideas. I understand that the project evolved in parallel to Spree at some point, but it hasn't been a rewrite, it's an evolution and wouldn't make things easier to read, write or understand to change the namespace.

An example of fork of a known project which kept the namespace of the original project is OpenProject which is a fork of Redmine and both are doing pretty well as is.

dustineichler commented 5 years ago

@patleb "implementation details" and stakeholder issues aren't mutually exclusive. No one is disputing this is a cosmetic update. No one is disputing potential impact. No one is saying we're doing it for the sake of it. We are looking for better anecdotes in this discussion than, "could potentially do more harm than good" and "i don't see any advantages".

Can you speak to some of the work you're doing and how this change might affect those projects.

patleb commented 5 years ago

"implementation details" and stakeholder issues aren't mutually exclusive

In what way if I might ask? I'm talking about this specific issue, not about all those in existence. In this case, I'm pretty sure this is irrelevant: when was the last time a stakeholder spoke up about how you name your classes or how some files are named?

No one is disputing this is a cosmetic update

Yes, sorry to say, but I doubt that you were refering to a "necessary" cosmetic touch here:

Obviously this would be a big lift, but necessary...

Also, I don't know why you're saying No one is saying we're doing it for the sake of it, you're contradicting the intention of your previous statement about cosmetic: how is cosmetic changes isn't for the sake of changing? If you ignore the stakeholder matter, then no other advantages has been brought up. Also, I tried to come with some, but failed to see something better than "it just makes sense to have the project name as the same namespace": I know it would be nice, but I really think it's too late for this (by maybe 5 years).

We are looking for better anecdotes

Really?! So basically, what I said about Spree meaning something about the code provenance, the fact that some teams use both Solidus and Spree, the quasi-identical data modeling (which you can verify by yourself by diffing the generated Spree and Solidus schema.rb), the "not helping" about understanding code and the example of a successful project with the same scenario as Solidus is worth nothing?

Can you speak to some of the work you're doing and how this change might affect those projects.

What are you trying to say here? You want me to justify why I should be heard and if it's worth it? If this isn't your intention, then I suggest that you rephrase this line in the future by something like "I think I might not see how it could affect some projects, can you tell me how this change could affect what you're working on?". For which I might happily answer:

Well, our main product supports both Spree and Solidus and we take advantage of what's shared between both project to simplify migration from one or the other if the client wishes to. Also, changing the namespace will add a headache for migrating data, every model using polymorphism and STI will be impacted: you cannot just rename the code, the data will have to be migrated as well. Finally, all extensions will need to be migrated and we'll have to modify every extension we have that we share with both Spree and Solidus: it'll add an architectural indirection by either using some kind of classes/constants aliases or extracting into modules, we'll have two sets of migrations, etc.

Changing the namespace will break all applications out there, not just mine and, as I said, I would like to come up with some advantages, but there is none of interest I could come up with.

As a last consequence, changing the namespace will touch basically every files and a high percentage of lines as well. So as someone who works through the library and use git comments to understand some decisions and how some parts are interrelated, it'll be quite inconvenient when I have to always jump several commits behind the big namespacing refactor to check what was changed and for which reason.

To sum up, I just think it's too late to make this kind of change, it's just not worth it (unless you ignore all the points I brought up... again).

jacobherrington commented 5 years ago

Those are good points @patleb. Thanks for sharing.

I tend to agree with Jared, this isn't extremely important.

The only benefit I see is trying to make it easier for those assessing Solidus and Spree to more clearly understand that they are not necessarily interchangeable (despite being very similar).

As I said before, this discussion is definitely worth having.

dustineichler commented 5 years ago

salient point here is and has been data migration. i don't see this as an insurmountable problem. providing a solution to help with data migration might be one way to go, but updating the implementation shouldn't be impedance to advancing an idea.

patleb commented 5 years ago

@jacobherrington, thx, no problems. I can understand how this might help if you assume that assessment includes reading the source code, but I've rarely seen it done this way (although, I personally do it, but more as a curiosity than an obligation at the first steps). Most of the time, it starts with the getting started and playing with the demo. So, as a side note, if differentiation is important, I would tend to think that doing it through the code isn't the most effective/rewarding place to do it (it may or may not help).

@dustineichler, I understand that you're on the defensive because I'm opposed to the idea for reasons I've brought up, but please try to understand what I'm saying because this is exactly what I'm doing "advancing the idea", just not in the direction you would like it to be. You're ignoring or minimising what I'm saying as if it was of no consequences. I'm not saying it's complicated to migrate data, I'm saying everyone will have to do it if goes forward.

salient point here is and has been data migration

No, there are other points I've talked about. Can you please give some advantages of your own on this subject? Because you pretty much proposed the idea, but just dismissed any disadvantages brought up as if the issue should not be challenged.

dustineichler commented 5 years ago

Proposing:

patleb commented 5 years ago

These points are disadvantages disguised as solutions to the consequences: they are adding to the cost, they are not bringing any benefits. It's a little bit concerning that you cannot propose any advantages whatsoever or maybe I haven't been clear enough. A good example of advantage is what @jacobherrington proposed: it would have the benefit to clarify that Solidus is an independent entity from Spree if the code is assessed by stakeholders.

Anyway, I think I made my points clear, I have no more time for this issue, I'll let the core team discuss this. Have a nice day!

jacobherrington commented 5 years ago

Thanks so much for your thoughts @patleb and @dustineichler. I prefer having these conversations in the open so people will know why we make the decisions we make :)

patleb commented 5 years ago

Sure, no problem :)

tvdeyen commented 5 years ago

I don’t see any advantage in changing the namespace either. All points @patleb brought up are valid concerns and no pro argument can circumvent them.

There are far more pressing problems to solve for us. This one is not on my list.

dustineichler commented 5 years ago

Closing. No momentum on this.

kennyadsl commented 5 years ago

Work done in #3364 outlines a possible realistic solution for this problem. With that PR's approach, which it's still a spike, we should be able to rename the namespace from Spree to Solidus without breaking stores and extensions code, printing deprecation warnings that explain how to move and rename files to be compliant with Solidus core.

Yesterday, we re-discussed this topic within the Core Team and, although we think this is a great solution, there are other things to take into account:

The main motivation behind this change is mainly to don't confuse new users that do not know what Spree is but these things above would only create more confusion since they are very likely to happen at least one time to everyone who will choose Solidus. They'd still have to understand the difference between the two things but this way it's just more hidden and cryptic.

That said, despite the great work done by @aldesantis (thanks!), we think it's still better to keep the Spree namespace and try to transform this "weakness" into a Solidus strength. We have the opportunity to explain to new users where Solidus comes from, a bit of history, and how it is different from Spree. We just need to find the best place where to do that, maybe a prominent page in our guides?