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

chore (drivers): extract drivers to a separate package #1332

Merged
merged 37 commits into from
Jun 26, 2024

Conversation

kevin-dp
Copy link
Contributor

@kevin-dp kevin-dp commented Jun 6, 2024

No description provided.

Copy link

linear bot commented Jun 6, 2024

@kevin-dp kevin-dp changed the base branch from main to kevindp/vax-1887-extract-cli June 6, 2024 12:27
@kevin-dp kevin-dp force-pushed the kevindp/vax-1903-extract-drivers branch 6 times, most recently from b4deb74 to 13245fd Compare June 6, 2024 13:01
@kevin-dp kevin-dp force-pushed the kevindp/vax-1887-extract-cli branch from d2f8080 to 2ebe05b Compare June 24, 2024 11:45
Base automatically changed from kevindp/vax-1887-extract-cli to main June 25, 2024 13:45
@kevin-dp kevin-dp force-pushed the kevindp/vax-1903-extract-drivers branch 8 times, most recently from 362ed3a to 2d1f8a3 Compare June 26, 2024 07:25
Copy link
Contributor

@msfstef msfstef left a comment

Choose a reason for hiding this comment

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

Looks great, nice work!

My only concern is with the extraction of some utilities, like the QualifiedTablename class, parseTableNames, and parseDate, which end up being core to the client as well but they belong to the drivers package, despite them only being used in tests or not used at all in the whole of the driver package.

I wonder if, since they are so few as well, we might benefit from keeping them in the client and re-implementing thinner abstractions for the drivers only?

components/drivers/src/util/parser.ts Outdated Show resolved Hide resolved
components/drivers/README.md Outdated Show resolved Hide resolved
…erwise we get subtle type compatibility issues.
… the compiled dist/bin.js file because pnpm install would fail if dist/bin.js does not exist but in order to build dist/bin.js we first need to pnpm install.
…bleName and parseTablenames to ts-client package
@kevin-dp kevin-dp force-pushed the kevindp/vax-1903-extract-drivers branch from 0fd9737 to ee33c9e Compare June 26, 2024 12:57
@kevin-dp kevin-dp merged commit af6a7be into main Jun 26, 2024
36 checks passed
@kevin-dp kevin-dp deleted the kevindp/vax-1903-extract-drivers branch June 26, 2024 13:07
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.

2 participants