glossier / solidus_retail

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

Add product/variant operations #35

Closed thisiscab closed 8 years ago

thisiscab commented 8 years ago

This PR encapsulate the logic used to UPDATE / DESTROY / CREATE products and variants on Shopify.

That way, the host app can simple call ProductOperations.update(spree_product: product) in order to update that spree product to Shopify.

The motivation behind this was to abstract what's the steps that are required to save/update/destroy solidus product to Shopify. Also, those method calls will be used in many places in the host app where we would want the sync those variant/product to Shopify.

thisiscab commented 8 years ago

@bryanmtl @braidn @DanielWright This is ready for CRing :)

braidn commented 8 years ago

Can we remove the merge bubbles created at the end for cleaner history?

thisiscab commented 8 years ago

How would you go about removing those merge bubbles? When I try to rebase they are not showing.

braidn commented 8 years ago

Yeah I don't see these merge bubbles locally at all which is weird. Maybe they are just Ghosts from GitHub's perspective

braidn commented 8 years ago

Cab can you also write a context block for people CRing here? Having a hard time understanding intent/motivation from your commits and the sentence used above. Thank you.

thisiscab commented 8 years ago

@braidn Modified the text, is that better?

braidn commented 8 years ago

@cabouffard will get on this either tonight or early AM.

braidn commented 8 years ago

@cabouffard we have a CR here and someone else's 👀 have seen the code. What's holding back the 🚢 ?

thisiscab commented 8 years ago

What's holding me back was I would have liked an answer about the question you have asked and I replied with a question about the naming of the Operation classes. We'll leave it as that for now, if you wish to change it I'm opened to PR request ;)