search

RobertSpread / spreadcart

simple plugin to display a cart for the spreadshop everywhere on your domain
1 stars 1 forks source link

Please create branch off master for a read-cart-from-api pull #26

Closed jtlapp closed 8 years ago

jtlapp commented 8 years ago

Hey Robert,

I've asked a few times now, but it might be getting lost in all the other discussion. I'm waiting for you to create a new branch off of master so I can do a pull request to give you my rewrite for pulling the shopping cart entirely from API. I don't want to do the pull request on your master branch, because you still need to get it working with the PHP proxy.

~joe

jtlapp commented 8 years ago

Just had a thought. You wanted to delay decoupling from the SpreadShop implementation, and I wanted to hasten it. Decoupling allowed me to solve the problem of showing an empty basket after checkout and allowed me to relax about other surprises that might be forthcoming.

But you may have your own reasons for preferring close coupling, such as ensuring that Spreadshirt itself could make efficient use of this implementation.

We can have it both ways if we hide the basket behind a generic interface whose implementation we can select. That is, an internal API. One implementation of the interface would be decoupled as I have done. Another would be as coupled as you prefer. We would do this by implementing two classes with the same methods. Any given installation of spreadCart would use just one of the classes.

RobertSpread commented 8 years ago

Hi there,

Sorry, I really lost the "please create a branch" in our discussions. Many apologies. I also do not have any issue decoupling it from the shop as it gives us more freedom on this entire topic. My only concern was, that we are not able to hand over a basketID to the shop and that this might cause issues in a future step of our little project here. The branch is created and I will merge it into master after I tried to understand everything in your pull request (this is not a sign of missing trust but rather a sign of "I really want to understand what is happening" - as said, I am no developer ;) ). I'll close this ticket for now.

jtlapp commented 8 years ago

Perfect. Thank you! I tend to be excessively communicative, so I'm used to things getting lost in the mix. I think people see my request for email in email/comment #2, but forget about it by the time they get to #192.