search

thaiphan / magento-s3

Use Amazon S3 as the file storage solution for your Magento store
MIT License
64 stars 22 forks source link

Replace Zend_Service_Amazon_S3 with official AWS SDK. #17

Open colinmollenhour opened 6 years ago

colinmollenhour commented 6 years ago

This isn't tested yet but would like to get your feedback.

Thanks!

thaiphan commented 5 years ago

Hey @colinmollenhour,

I'm a big fan of your work!

I thought I wrote a reply a while ago but I guess I forgot to submit it!

Thanks for this pull request! However, I don't know if I can merge this. I've found that a lot of people still don't use Composer to manage their Magento dependencies and it will be a nightmare to tell them that the extension won't work. I've also been meaning to eventually upload this to the Magento Marketplace and I dunno if using Composer will work with that use-case.

What are your thoughts?

Regards,

Thai

colinmollenhour commented 5 years ago

Thanks for the feedback.

For the marketplace you could package it up after running "composer install" locally so that the files are all included, but currently they still need to be include or linked up with the autoloader somehow. For github users that don't use composer you could add some additional installation steps such as "download AWS SDK here, extract, etc.." although I concede that is not ideal. Unfortunately since Magento 1 does not work with composer out of the box it is kinda messy, but I'm pretty sure you don't want to just include the entire Amazon SDK in your repo but that would be an option as well.

To avoid breaking existing installations you could merge the new version into a new branch such as "aws-sdk" (I'd recommend making this the new default branch at that point) which would be used for composer users and the packaged versions. Modman users using master and Composer users using "dev-master" would continue to get the old version so at least their installations wouldn't break. The new version could be a 2.x release so that 1.x remains on the Zend libs. Just some ideas.

There are some bugs with the code still so don't merge it, just wanted to get your feedback to see if this was a direction you were interested in taking it. If you are still interested I can push up the fixes so that the code works 100% but managing the branchs and release strategy will still be up to you. Let me know what you think and thanks for sharing your work!

thaiphan commented 5 years ago

Hi @colinmollenhour,

Yeah, I am happy to maintain two separate branches:

How do you feel about that?

Regards,

Thai

colinmollenhour commented 5 years ago

Sounds good! Will update this branch with a more thoroughly tested PR in the next week or two.

colinmollenhour commented 5 years ago

Pushed another commit. Still not tested thoroughly yet but I think it's just about there.