Skip to content

fix adapter string config #3538

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

Closed

Conversation

jeacott1
Copy link

@jeacott1 jeacott1 commented Feb 20, 2017

allows push adapter to be properly configured via string config

@flovilmart
Copy link
Contributor

Could you add a test that exemplify that behavior?

@@ -23,6 +23,9 @@ export function loadAdapter(adapter, defaultAdapter, options) {
if (adapter.default) {
adapter = adapter.default;
}
if(options.adapter) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Alignment is off

Copy link
Author

Choose a reason for hiding this comment

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

yeah - its a pain to test locally (windows) - so for tiny changes its easiest to just PR them and let travis let me know if its ok.
just trying a variation, and if its ok I'll add a test.

atm you cant configure a push adapter via string, which is a pain.

@jeacott1 jeacott1 force-pushed the fix-adapter-string-loader branch from 88e5649 to 0e11a7f Compare February 20, 2017 03:14
@facebook-github-bot
Copy link

@jeacott1 updated the pull request - view changes

@jeacott1 jeacott1 force-pushed the fix-adapter-string-loader branch from 0e11a7f to 52b07f6 Compare February 20, 2017 03:20
@facebook-github-bot
Copy link

@jeacott1 updated the pull request - view changes

@flovilmart
Copy link
Contributor

There is the test:win command that is specifically designed for windows.

@flovilmart
Copy link
Contributor

Also, if you're using the CLi, you'll note that the parser is an objectParser not a moduleParser (unlike emailAdapter options). I suggest you open an issue first, with the proper error you encounter.

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.

3 participants