-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: add enum values to MySQL object of property decorator #10605
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
base: master
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 9876716776Details
💛 - Coveralls |
f45e140
to
5016340
Compare
Signed-off-by: Muhammad Aaqil <[email protected]>
5016340
to
375178c
Compare
@samarpanB, since you work with the mysql connector closely, could you please take a look at this PR? Thanks. |
Okay |
@aaqilniz can you please link it with an issue ? That ways we can understand what exact problem its resolving. |
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.
@aaqilniz, thanks for the PR. Sorry that it's taking so long to have this reviewed.
I'm wondering if it fits better to have this discovery code in the MySQL connector.
length: null, | ||
precision: null, | ||
scale: null, | ||
mysql: { |
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.
There's acceptance in https://github.com/loopbackio/loopback-next/tree/master/acceptance/repository-mysql. It might be better to keep the database/connector specific tests there.
Hi, @dhmlau. I have moved this change to loopbackio/loopback-datasource-juggler#2420. I think adding these changes here is not a good idea. This might cause problems for other connectors. I have added these changes where this db-specific field (mysql, postgresql or memory) is already being added in the above PR. |
Checklist
npm test
passes on your machinepackages/cli
were updatedexamples/*
were updated👉 Check out how to submit a PR 👈