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

feat: add DBStructure schema #83

Merged
merged 5 commits into from
Nov 14, 2024
Merged

feat: add DBStructure schema #83

merged 5 commits into from
Nov 14, 2024

Conversation

sasamuku
Copy link
Member

@sasamuku sasamuku commented Nov 12, 2024

Summary

This is an experimental schema and is likely to change in the near future.
As a first step, referencing drawDB, which offers functionality similar to our service.
https://github.com/drawdb-io/drawdb/blob/main/src/data/schemas.js

Related Issue

Changes

Testing

Other Information

Please note that some drawDB schemas have been removed for minimal start.

@sasamuku sasamuku force-pushed the add_db_structure_schema branch from cec3f3d to 2533272 Compare November 12, 2024 10:18
@sasamuku sasamuku marked this pull request as ready for review November 12, 2024 10:32
@sasamuku sasamuku requested a review from a team as a code owner November 12, 2024 10:32
@sasamuku sasamuku requested review from hoshinotsuyoshi, FunamaYukina, junkisai and MH4GF and removed request for a team November 12, 2024 10:32
Copy link
Member

@hoshinotsuyoshi hoshinotsuyoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in packages/db-structure/src/schema/index.ts look good, and it seems like updating just this file should be sufficient. What do you think?

frontend/package-lock.json Outdated Show resolved Hide resolved
frontend/packages/db-structure/tsconfig.json Outdated Show resolved Hide resolved
@@ -1,9 +1,51 @@
import * as v from 'valibot'

const tablesSchema = v.object({})
const fieldSchema = v.object({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

@MH4GF MH4GF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally no problem 👍🏻
But I'm only commenting on the parts that would change the implementation significantly in the future.

frontend/packages/db-structure/src/schema/index.ts Outdated Show resolved Hide resolved
frontend/packages/db-structure/src/schema/index.ts Outdated Show resolved Hide resolved
@sasamuku sasamuku force-pushed the add_db_structure_schema branch from 2533272 to 9d85a7d Compare November 14, 2024 02:53
@sasamuku sasamuku requested a review from MH4GF November 14, 2024 03:33
Copy link
Member

@MH4GF MH4GF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the adjustment!
I only commented on the details, but I will leave it to you!

deleteConstraint: v.string(),
})

const tablesSchema = v.record(v.string(), tableSchema) // Key is table name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits: If you are going to make comments like this, it is better to use the type alias practice:

Suggested change
const tablesSchema = v.record(v.string(), tableSchema) // Key is table name
const tableNameSchema = v.string()
const tablesSchema = v.record(tableNameSchema, tableSchema)

Of course, it is also used for startTableName, etc.
Improves readability by providing a link between which values are referenced and where.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, added type alias:
6cde75d

@sasamuku sasamuku added this pull request to the merge queue Nov 14, 2024
Merged via the queue into main with commit 8c44a77 Nov 14, 2024
7 checks passed
@sasamuku sasamuku deleted the add_db_structure_schema branch November 14, 2024 07:04
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.

3 participants