glossier / solidus_retail

Solidus Extension to Support Retail Operations
BSD 3-Clause "New" or "Revised" License
8 stars 4 forks source link

How can we better express the dependency on spree_product_assembly? #37

Open DanielWright opened 7 years ago

DanielWright commented 7 years ago

Having a dependency expressed in the Gemfile makes it awkward and difficult for host applications to reconcile dependency conflicts, because anything in the Gemfile isn't available to RubyGems.

Some ideas:

I'm inclined toward the first option, though it would probably require some work to extract any Glossier-specific implementation.

Any other ideas?

DanielWright commented 7 years ago

Oh wait, solidus_product_assembly is already a thing. Why don't we add this as a runtime dependency in the gemspec?

thisiscab commented 7 years ago

I am unsure how all of this actually work.

If we add https://github.com/solidusio-contrib/solidus_product_assembly as a dependency in our gemspec and we were to want to use our fork instead in our host-app how would that work out?

DanielWright commented 7 years ago

This is one reason why we shouldn't use forks for active extensions.

thisiscab commented 7 years ago

In the unfortunate case that we do, what would you suggest?

DanielWright commented 7 years ago

A dependency in a gemspec means only that Bundler will look for a matching gem in the Gemfile. A fork should satisfy, as long as we're not really strict about gem version in the dependency declaration.

On Sep 27, 2016, 5:52 PM -0400, Charles-André Bouffard notifications@github.com, wrote:

In the unfortunate case that we do, what would you suggest?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub (https://github.com/glossier/solidus_retail/issues/37#issuecomment-250010315), or mute the thread (https://github.com/notifications/unsubscribe-auth/AAnPAVEhtqW0g_pxV2q5g0K9tGsHhHHBks5quZAvgaJpZM4KHvmX).

thisiscab commented 7 years ago

Ugh, in our fork of the gem, products may have many parts. But in the original gem, products doesn't have any parts but variant does, which kind of changes the implementation.

This is not a trivial fix as I would have expected.

thisiscab commented 7 years ago

The problem I had expressed:

Instead of a product having parts (which is the case in our fork), variants have parts (in the original implementation). We'll have to figure something out.

Was not true. I just had made a dummy mistake :)

braidn commented 7 years ago

I feel like this gem should have no requirements on the product assembly gem. Just hooks into the importer where people who are using the assembly gem can hook in and glue both gems together in the host app.

thisiscab commented 7 years ago

You would move everything related to the bundle exporting to the host app? I would agree that this is a bad idea for the gem requirement but I think we should fix it post-launch, watcha think?

braidn commented 7 years ago

I don't know if we need to change it post launch. We should slowly move the glue code to the host app (Glossier) and remove the entire requirement on the assembly gem for both uploading to Shopify and Order sync from Shopify.

thisiscab commented 7 years ago

I think that in this scenario everything works even though it's not considered the best implementation. I don't want to move everything at Glossier has this would impede on a lot of stuff. The QA has already been "started" and changing how the bundled products are being imported from this gem to Glossier may bring difficulties / problems.

But if you would like to open a PR what would satisfy that? I'm down! :)

bryanmtl commented 7 years ago

Glossier's fork of the product assembly gem has a number of differences that will be difficult to reconcile with the "official" solidus version. That Gem, FWIW, has its own set of problems and I don't think it's widely used in the community.

Once again, I think it's vital to have this working for Glossier. If we want to make a more generic version after the launch of retail, these extra dependencies can be removed.

We are dealing with a significant number of bundled (or assembled) products in our catalog, and this has to be a core feature of our retail gem.