-
Notifications
You must be signed in to change notification settings - Fork 182
Add loopback-connector into peer dependencies #246
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
Add loopback-connector into peer dependencies #246
Conversation
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
Can one of the admins verify this patch? |
2 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
@bobbysmith007 I can't find your branch in https://github.com/AccelerationNet/loopback-datasource-juggler/branches. Any reason? Could you please rebase your branch and fix the commit linter? Here is the linting error:
Linting rule:
Could you also submit a PR to fix this in loopback-datasource-juggler? |
Here is the other PR: loopbackio/loopback-datasource-juggler#1326 https://github.com/AccelerationNet/loopback-datasource-juggler/tree/fix-connector-dependency I can amend the patch on the morning but I'm away from my workstation for the day |
0661c6c
to
6e70466
Compare
@bobbysmith007 Ah sorry I was trying to look for branch |
@slnode test please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Why was this reverted? |
There were some issues with the installation of |
* Add docker setup (#256) (Sakib Hasan) * Allow explicit numeric datatype (#254) (Sakib Hasan) * Allow non-id serial properties (#198) (zbarbuto) * Revert PR #246 (#248) (Sakib Hasan) * Add loopback-connector as peer dependencies (#246) (Russ Tyndall) * Fix operations on ended transactions (zbarbuto) * dbdefaults: Cleanup InvalidDefault def after test (Kevin Delisle) * Reuse the data source to avoid too many clients (Raymond Feng)
Description
as a submodule by loopback-connector-postgres when used in a
parent loopback project. Because we reference objects/constructors
out of loopback-connector, and ask isinstance style questions of
them, they need to resolve to the same constructor
EG:
pass it to buildDefinition (a where clause), but it is not the same
ParameterizedSQL object (even if loaded from the same lib/version),
because it is in the sub node_packages
NB: this patch should probably be mirrored in loopback-datasource-juggler
To effectively share object models, everyone needs to load the same
library from the same spot.
loopback loads loopback-datasource-juggler (and should also load
loopback-connector) and any particular app will choose the backend
loopback-connector-postgres, we want all of these libs to refer to the
same objects
Testing
Since this only happens in concert with other libraries and dependency chains I was not sure how to add a unit/integration test to this project about it. That said "peerDependencies" seems to have been exactly created for this purpose
Related issues
Checklist
guide