Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make library files individually require-able #2627

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

mamhoff
Copy link

@mamhoff mamhoff commented Oct 21, 2017

We would like to be able to use parts of ActiveMerchant without having to load hundreds of Gateways and their dependencies.

This is a first step towards individually require-able library files.

I have not checked whether ActiveSupport dependencies are in those files. They probably are, but for all the Rails users, ActiveSupport will be already loaded.

In order to not use ActiveMerchant's support classes outside
of ActiveMerchant, it's nice to have files explicitly require
their dependencies.

As now ActiveMerchant::Billing::Response will load its dependencies,
there's no need to explicitly require them in the activemerchant/billing.
In order to not use ActiveMerchant's support classes outside
of ActiveMerchant, it's nice to have files explicitly require
their dependencies.

As now ActiveMerchant::Billing::CreditCard will load its dependencies,
there's no need to explicitly require them in the activemerchant/billing.
This is to allow requiring files using this method
without loading all of ActiveMerchant.
Pleas eenter the commit message for your changes. Lines starting
This module wants to be individually require'able.
@bdewater
Copy link
Contributor

I think this is a great idea, and a fair tradeoff that users opting into this bring their own active_support require if needed. @jasonwebster @davidsantoso WDYT?

@bpollack
Copy link
Contributor

bpollack commented Jun 24, 2018

@bdewater I'm not conceptually averse to this, but I think we should probably have a Slack convo before proposing code. I'm in particular concerned about how this would interact with some of our internal stuff. Even if it's a problem, it might be worth merging, but since this solves a non-problem from our end, I'd like to figure out what the benefits are.

Implicit in which: is this something you're interested in/want to actively pursue?

(This is separate from our convo of deprecating unused gateways, since that does have a genuine value proposition that's worth the risk.)

@bpollack bpollack added gateway/feature Adds a new feature to an existing gateway improvement and removed gateway/feature Adds a new feature to an existing gateway labels Jun 24, 2018
@bpollack
Copy link
Contributor

I think my previous comment is way off-base. Having looked at this more carefully, I think it'd be great to land, and would help move towards supporting #2923

@mamhoff
Copy link
Author

mamhoff commented Jul 11, 2018

I've kinda moved on from this task, and won't spend time on this PR right now. Feel free to rebase, re-open, squash. For the record: I think this would still be a good step for the library to take.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants