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

Feature/refactor connector base #118

Closed
wants to merge 17 commits into from
Closed

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented May 23, 2014

Remove Connector and BaseSQL, they were moved to loopback-connector.

Add ModelBuilder.prototype.defineValueType that should be used by connectors like loopback-connector-mysql to register new schema types.

Requires loopbackio/loopback-connector#2.

/to @raymondfeng @ritch please review

strongloop/loopback#275

Raymond Feng and others added 17 commits May 16, 2014 10:07
Modify ValidationError constructor to include the model name and
a human-readable representation of the validation errors (messages)
in the error message.

Before this change, the message was pointing the reader
to `err.details`.  Most frameworks (e.g. express, mocha) log only
`err.message` but not other error properties, thus the logs were
rather unhelpful.

Example of the new error message:

    The `User` instance is not valid. Details: `name` can't be blank.
…ror-toString

validations: include more details in `err.message`
Remove references to Connector and BaseSQL, connectors should require()
loopback-connector instead of loopback-datasource-juggler.
Add a shortcut for registering a new value type.

The current implementation registers the type in the singleton registry
`ModelBuilder.schemaTypes`.

The API should allow us to to change the implementation to register
the type in the scope of ModelBuilder instance only.
@slnode
Copy link

slnode commented May 23, 2014

Test Failed. To trigger a build add comment - ".test\W+please"
Refer to this link for build results: http://ci.strongloop.com/job/loopback-datasource-juggler/446/

@bajtos
Copy link
Member Author

bajtos commented May 23, 2014

The build if failing because loopback-connector does not have a CI job yet and is not published to npmjs.org.

@rmg Could you please create the CI job for loopback-connector?

@rmg
Copy link
Contributor

rmg commented May 23, 2014

@bajtos I've added loopback-connector to CI, but it probably won't help much until its initial PR is merged into master.

@rmg
Copy link
Contributor

rmg commented May 23, 2014

test please

@slnode
Copy link

slnode commented May 23, 2014

All is well
Refer to this link for build results: http://ci.strongloop.com/job/loopback-datasource-juggler/447/

@ritch
Copy link
Contributor

ritch commented May 28, 2014

Does this need to be rebased on master? The diff seems a bit large for what we are doing here. I see a ton of unrelated changes to docs.

@bajtos
Copy link
Member Author

bajtos commented May 29, 2014

Rebased on top of 2.0, replaced loopback-connector:^1.0 with loopback-connector:1.x and submitted in a new pull request #127. Let's move the discussion there.

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.

5 participants