boomerdigital / solidus_flexi_variants

BSD 3-Clause "New" or "Revised" License
15 stars 31 forks source link

Separate into3 engines #3

Closed masonjeffreys closed 6 years ago

masonjeffreys commented 7 years ago

Following the Solidus convention, this branch is a work in progress to separate the solidus_flexi_variants gem into: core, backend, and frontend engines so that they can be used independently.

dhonig commented 7 years ago

Thanks for this. Give us a few days to review and comment. Lots of people out for the holiday weekend so expect to hear something next week.

masonjeffreys commented 7 years ago

Sounds good. Thanks Daniel! Jeff

On Tue, Aug 29, 2017 at 2:07 AM, Daniel Honig notifications@github.com wrote:

Thanks for this. Give us a few days to review and comment. Lots of people out for the holiday weekend so expect to hear something next week.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/boomerdigital/solidus_flexi_variants/pull/3#issuecomment-325564445, or mute the thread https://github.com/notifications/unsubscribe-auth/ACFCIYEFcmP0Q-MTU5FBMrlO83uOBGAlks5sc6qVgaJpZM4PFRO2 .

masonjeffreys commented 6 years ago

Just a ping on this.

dhonig commented 6 years ago

@acreilly can you take a look at this?

acreilly commented 6 years ago

Is there a way to run this through Travis?

masonjeffreys commented 6 years ago

Ping on this. Happy to tackle some more work if it's a direction you all do want to move in.

dhonig commented 6 years ago

@masonjeffreys Thanks, let me have @eric1234 or @kenton comment on this before we spend more time on it. I did look at it last week and thought it made sense, but want some more eyes on it.

eric1234 commented 6 years ago

Looking through this I see some good cleanup :+1:

As to the splitting into three engines to mirror the organization structure of Spree/Solidus itself I don't really have a strong opinion. In theory someone could load just the core or just the core + backend but I don't really see any big gain in doing that. IMHO the three distinct engines don't offer any real advantage over just using namespaces (as it was already doing) and it slightly increases the amount of boilerplate.

That being said the same strategy is used in Spree/Solidus so I wouldn't even flinch if I saw this separation in a plugin. I don't think the three engines come with any real disadvantage so I wouldn't discourage it either.

masonjeffreys commented 6 years ago

Thanks for the comments @eric1234 . The reason I requested this change was that I wanted to use solidus_flexi_variants but I was not using a conventional Solidus frontend. Solidus_flexi_variants seemed to make assumptions that I was using the conventional solidus frontend (it assumed specific controller naming I believe).

Perhaps there is another, easier way to let folks use this gem with a custom solidus frontend.

masonjeffreys commented 6 years ago

All, Haven't seen much movement on this. Perhaps there's an easier way to achieve the same result? Maybe adding some optional config to be able to exclude the frontend pieces of the gem? I'm just looking for a way to use this with a custom frontend. Up for suggestions.

masonjeffreys commented 6 years ago

Closing this pull request in favor of a new approach that adds conditional statements to the frontend decorators so that people can use the gem without a standard solidus frontend.