magento / community-features

Magento Features Development is an Initiative to Allows Community Memebers Join to Development of Magento Features
46 stars 18 forks source link

Remove braintree/braintree_php from magento/magento2-base #243

Open andparsons opened 5 years ago

andparsons commented 5 years ago

Summary (*)

braintree/braintree_php is required in magento/magento2-base when the only module that relies on that dependancy is magento/module-braintree, which also has braintree/braintree_php as a dependancy

I am not composer requiring magento/product-community-edition but instead magento/magento2-base with a subset of Magento modules, which I believe is a better practice than using composer's replace property to remove unused dependancies.

I don't want braintree/braintree_php to be installed as none of the modules I'm using make use of it.

Examples (*)

N/A

Proposed solution

I am suggesting removing braintree/braintree_php from magento/magento2-base, but not from magento/product-community-edition or magento/product-enterprise-edition

This should allow for braintree/braintree_php to only be installed via composer if magento/module-braintree is part of the resolved dependancies, and will keep braintree/braintree_php in the root composer.json if composer requiring magento/product-*-edition or cloning from git.

Other dependancies like msp/recaptcha have the ideal setup, where they get removed from magento/magento2-base when the composer.json package is created.

I don't believe I can create a pull request for this from the public GitHub repo.

m2-assistant[bot] commented 5 years ago

Hi @andparsons. Thank you for your report. To help us process this issue please make sure that you provided the following information:

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.3-develop instance - upcoming 2.3.x release

For more details, please, review the Magento Contributor Assistant documentation.

@andparsons do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?


andparsons commented 5 years ago

Hi @andparsons. Thank you for your report. To help us process this issue please make sure that you provided the following information:

  • [x] Summary of the issue
  • [x] Information on your environment
  • [x] Steps to reproduce
  • [x] Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.3-develop instance - upcoming 2.3.x release

For more details, please, review the Magento Contributor Assistant documentation.

@andparsons do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

I confirm that I am able to reproduce the issue on vanilla Magento. It's just composer bundle changes after all.

magento-engcom-team commented 5 years ago

Hi @andparsons. Thank you for your request. I'm working on Magento 2.3-develop instance for you

magento-engcom-team commented 5 years ago

Hi @andparsons, here is your Magento instance. Admin access: https://i-25189-2-3-develop.instances.magento-community.engineering/admin Login: admin Password: 123123q Instance will be terminated in up to 3 hours.

andparsons commented 5 years ago

Hi @andparsons, here is your Magento instance. Admin access: https://i-25189-2-3-develop.instances.magento-community.engineering/admin Login: admin Password: 123123q Instance will be terminated in up to 3 hours.

Not needed for this type of issue but that was nice and quick.

m2-assistant[bot] commented 5 years ago

Hi @BrentRobert. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:


BrentRobert commented 5 years ago

Hi @andparsons, I created a pull request as I agree with your proposal.

joni-jones commented 5 years ago

@andparsons and @BrentRobert, please, don't forget that Magento can be installed from the Git, how in this case Braintree SDK will be loaded if you remove it from the root composer json?

BrentRobert commented 5 years ago

I kinda forgot about that, my bad

andparsons commented 5 years ago

@joni-jones and @BrentRobert, thanks for your input guys, but as I said this is a change needed when bundling for magento/magento2-base, which I believe the process for that is not public.

Hopefully someone from the core team is still going to look at this issue.

m2-assistant[bot] commented 5 years ago

Hi @engcom-Delta. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Delta commented 5 years ago

There is definitely no possibility to make pull request for this from the public GitHub repo. Marked as Feature Request cc @sdzhepa

hostep commented 5 years ago

@sdzhepa: can we somehow open up the script/repo to the public for the magento2-base package and/or composer metapackage? There are a bunch of tiny little tweaks the community wants to accomplish (https://github.com/magento/magento2/issues/22359 comes to mind and there are some others I think). These aren't very high priority. But if the community can not help out, it will probably never happen if only the core team will pick it up when they feel like it. Thanks!

sdzhepa commented 4 years ago

Hello @hostep

Unfortunately, after so long delay and multiple times raising this question internally, I did not receive a clear answer for

"can we somehow open up the script/repo to the public for the magento2-base package and/or composer metapackage?".

In general, as I understood there are a couple of difficulties and policies that do not allow us to do it.

I will move this ticket into the community-feature repository to be considering one more time there

hostep commented 4 years ago

Thanks for trying to pursue this @sdzhepa!

nj commented 3 years ago

Seems like it has been handled in https://github.com/magento/magento2/commit/7400615 :+1: