-
Notifications
You must be signed in to change notification settings - Fork 14
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 initial tbls parser #577
Conversation
🦋 Changeset detectedLatest commit: a580fb6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@@ -7,6 +7,7 @@ const nameToFormatMap: Record<string, SupportedFormat> = { | |||
'schema.rb': 'schemarb', | |||
schemafile: 'schemarb', | |||
'prisma.schema': 'prisma', | |||
'schema.json': 'tbls', |
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.
📝 schema.json
is a general name.
I think it's alright to handle it when namespace conflicts in Liam.
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.
This is troubling, but I think this is not a problem because it is just inference logic.
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 👍🏻
"@liam-hq/db-structure": patch | ||
"@liam-hq/cli": patch |
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.
Since last Friday, we've been shipping new features as minor
instead of patch
. https://github.com/liam-hq/liam/releases/tag/%40liam-hq%2Fcli%400.2.0
For this PR, would minor
be appropriate? Or should we consider this an initial
implementation? @MH4GF @sasamuku
"@liam-hq/db-structure": patch | |
"@liam-hq/cli": patch | |
"@liam-hq/db-structure": minor | |
"@liam-hq/cli": minor |
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.
@hoshinotsuyoshi Thanks!
I thought that patch
was still good enough for now.
The intent is available to the user but unstable and treated as experimental.
I would like to make it minor
at a time when we have completed some implementation!
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.
The intent is available to the user but unstable and treated as experimental.
That makes sense! Thanks for the clarification. @MH4GF
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.
Thanks, I understood 👍
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!
Summary
schema.json
ERD Web:
https://liam-erd-b9jxja3n1-route-06-core.vercel.app/erd/p/github.com/k1LoW/tbls/blob/v1.81.0/sample/mysql/schema.json
CLI:
Related Issue
Changes
Testing
Other Information