Skip to content

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aaqilniz
Copy link
Contributor

@aaqilniz aaqilniz commented Jul 10, 2024

Checklist

  • DCO (Developer Certificate of Origin) signed in all commits
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

@coveralls
Copy link

Pull Request Test Coverage Report for Build 9876716776

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 54.772%

Totals Coverage Status
Change from base Build 9858607965: 0.0%
Covered Lines: 9566
Relevant Lines: 12465

💛 - Coveralls

@aaqilniz aaqilniz closed this Aug 25, 2024
@aaqilniz aaqilniz reopened this Aug 25, 2024
@aaqilniz aaqilniz force-pushed the fix/enum-values-mysql branch from f45e140 to 5016340 Compare August 25, 2024 17:47
@aaqilniz aaqilniz force-pushed the fix/enum-values-mysql branch from 5016340 to 375178c Compare August 25, 2024 17:48
@aaqilniz aaqilniz marked this pull request as ready for review August 25, 2024 17:50
@dhmlau
Copy link
Member

dhmlau commented Nov 28, 2024

@samarpanB, since you work with the mysql connector closely, could you please take a look at this PR? Thanks.

@samarpan-b
Copy link

Okay

@samarpanB
Copy link
Contributor

@aaqilniz can you please link it with an issue ? That ways we can understand what exact problem its resolving.

Copy link
Member

@dhmlau dhmlau left a 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: {
Copy link
Member

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.

@aaqilniz
Copy link
Contributor Author

aaqilniz commented Jul 1, 2025

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.

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