Skip to content

Support custom connector #199

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

Merged
merged 1 commit into from
Jun 15, 2016
Merged

Support custom connector #199

merged 1 commit into from
Jun 15, 2016

Conversation

jannyHou
Copy link
Contributor

@jannyHou jannyHou commented Jun 8, 2016

Connect to strongloop/loopback#2265

Now when we choose a custom connector that not included in available connectors, it returns error.
This pr is created for supporting custom connector, and print a message to remind user manually add config then.

@jannyHou
Copy link
Contributor Author

jannyHou commented Jun 9, 2016

@bajtos Please review, thanks:)

@jannyHou
Copy link
Contributor Author

jannyHou commented Jun 9, 2016

@slnode test please

var modelGen = givenDataSourceGenerator();
helpers.mockPrompt(modelGen, {
name: 'test-custom',
customConnector: 'loopback-component-storage',
Copy link
Member

Choose a reason for hiding this comment

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

Please use a name that will never become a connector, e.g. async or lodash. I think the storage component is sort of connector-like.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. "a name that will never become a connector" makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

async is already there, i use lodash then

@bajtos bajtos removed their assignment Jun 10, 2016
@bajtos
Copy link
Member

bajtos commented Jun 10, 2016

@jannyHou reviewed, PTAL

@bajtos
Copy link
Member

bajtos commented Jun 13, 2016

LGTM 👏 Please rebase on top of the current master and squash the commits.

if (!pkg) return;
}

var npmModule = pkg.name || connector || this.connector;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bajtos When I check the code again, I think it should be
var npmModule = pkg.name || this.connector instead of
var npmModule = pkg.name || connector || this.connector

The connector is either undefined or an object, not a string. Thought?

Copy link
Member

Choose a reason for hiding this comment

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

The connector is either undefined or an object, not a string. Thought?

Ah, that's actually expected, see L200:

var connector = this.availableConnectors[this.connector];

I think it should be

var npmModule = pkg.name || this.connector;

Makes sense to me! 👍

@jannyHou jannyHou force-pushed the fix/custom-connector branch from 0d3ec2f to 28ad27a Compare June 13, 2016 18:05
@jannyHou
Copy link
Contributor Author

@slnode test please

@jannyHou jannyHou merged commit 13e28bb into master Jun 15, 2016
@jannyHou jannyHou removed the #review label Jun 15, 2016
@0candy 0candy deleted the fix/custom-connector branch June 17, 2016 16:09
@0candy 0candy restored the fix/custom-connector branch June 17, 2016 16:09
@jannyHou jannyHou deleted the fix/custom-connector branch July 15, 2016 18:19
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