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

feat(providers): add ability to register tax and fulfillment providers using medusa-extender annotations #146

Merged
merged 6 commits into from
Nov 23, 2022

Conversation

dwene
Copy link
Contributor

@dwene dwene commented Nov 21, 2022

No description provided.

@dwene
Copy link
Contributor Author

dwene commented Nov 22, 2022

@adrien2p ok I added some unit and integration tests! I'd like to start with these 2 fulfillment providers for now, but I'm setting up patterns here to be used again for payment, search, notification, file, oauth, and other future providers!

@dwene
Copy link
Contributor Author

dwene commented Nov 22, 2022

Also this closes #128

Copy link
Owner

@adrien2p adrien2p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very nice, Can I get you to resolve the codeQL comment as well and mine and it should be ready to merge 🚀


export type AllowedProviderInjectableOptions = Omit<ProviderInjectableOptions, 'subtype'>;
/**
* Mark a class as an entity to be used by the loader.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: update

import { customEventEmitter } from '../core';
import { customEventEmitter, GetInjectableOptions } from '../core';
import { registerProviders } from './providers.loader';
import { container } from 'webpack';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: rm

Copy link
Owner

@adrien2p adrien2p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🔥

@dwene
Copy link
Contributor Author

dwene commented Nov 22, 2022

Sweet! Let's ship it!

@adrien2p adrien2p merged commit e44ffcf into adrien2p:main Nov 23, 2022
@adrien2p
Copy link
Owner

@dwene i ve merged it. Can I get you to update the doc please?

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

Successfully merging this pull request may close these issues.

2 participants