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(*): support SQL output format #122

Merged
merged 43 commits into from
Mar 15, 2023
Merged

Conversation

maxdemaio
Copy link
Member

@maxdemaio maxdemaio commented Feb 23, 2023

This PR creates the functionality as discussed on #59 to the support the conversion of odict to SQL format. This will allow people to seed a database with odict data easily in all popular SQL dialects. This is thanks to @bokwoon95's help & project, sq.

The following commands are now available:

  • go run odict.go d -f postgres examples/example1.odict examples/output.sql
  • go run odict.go d -f sqlite examples/example1.odict examples/output.sql
  • go run odict.go d -f mysql examples/example1.odict examples/output.sql
  • go run odict.go d -f sqlserver examples/example1.odict examples/output.sql

I added these to the .vscode/launch.json file which allows us to test them in VS-Code! Very excited about this feature and would be glad to receive any feedback. Here is an example image of my seeded PostgreSQL database with odict data thanks to the ouput.sql file generated by the postgres command above:

image

@maxdemaio maxdemaio linked an issue Feb 23, 2023 that may be closed by this pull request
@Nickersoft Nickersoft changed the title 59 Support SQL Output Format feat(*): support SQL output format Feb 23, 2023
Copy link
Member

@Nickersoft Nickersoft left a comment

Choose a reason for hiding this comment

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

Yoooo this is awesome!! 🔥 So excited to see this coming together. I apologize in advanced for the flood of comments haha.

cli/app.go Outdated Show resolved Hide resolved
cli/app.go Outdated Show resolved Hide resolved
cli/dump.go Show resolved Hide resolved
cli/dump.go Outdated Show resolved Hide resolved
go/converters.go Show resolved Hide resolved
go/converters.go Outdated Show resolved Hide resolved
go/converters.go Outdated Show resolved Hide resolved
@maxdemaio
Copy link
Member Author

maxdemaio commented Feb 23, 2023

Yoooo this is awesome!! 🔥 So excited to see this coming together. I apologize in advanced for the flood of comments haha.

Thanks for all the comments!! I am going to make sure I address all of them. I'm going to be out of town this week/weekend, but will get to them as soon as I can 💪🏻 Appreciate how quickly you reviewed this! 😁

@maxdemaio
Copy link
Member Author

I now catch errors for invalid formats! But, I will not include this in the launch.json file since it's an invalid command. Now on execution of go run odict.go d -f fakeFormatLol examples/example1.odict examples/output.sql we get:

image

@maxdemaio
Copy link
Member Author

maxdemaio commented Feb 25, 2023

Also, feel free to let me know if you'd like me to add documentation to this. I'm not sure which takes preference, the gitbook repository or the docs folder in this repo. Either way, I'd be glad to do so!

Copy link
Member

@Nickersoft Nickersoft left a comment

Choose a reason for hiding this comment

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

LGTM! Just left some nits

cli/app.go Outdated
&cli.StringFlag{
Name: "format",
Aliases: []string{"f"},
Usage: "Specific output format of the dump (ODXML or SQL)",
Copy link
Member

Choose a reason for hiding this comment

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

Nit: "Specific" seems unnecessary here

cli/app.go Outdated
},
Before: cli.BeforeFunc(func(c *cli.Context) error {
s := c.String("format")
switch s {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: could probably be more concise as a simple if/else statement

@Nickersoft
Copy link
Member

Also sorry it's taken me so long to get back to this 😅

@maxdemaio
Copy link
Member Author

Also sorry it's taken me so long to get back to this 😅

No worries at all! I fixed the nits you suggested and will merge this in since the test cases still pass 😁 thank you for taking the time to review the PR. Very excited about this feature and I'd be glad to help on future ones 🎊 🚀

@maxdemaio maxdemaio merged commit 7dd99c5 into main Mar 15, 2023
@maxdemaio maxdemaio deleted the 59-support-multiple-output-formats branch March 15, 2023 12:42
@github-actions github-actions bot mentioned this pull request Mar 15, 2023
@github-actions github-actions bot mentioned this pull request Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Support multiple output formats
2 participants