8000 Add new command `convert` to convert SQL migrations to pgroll operations by kvch · Pull Request #685 · xataio/pgroll · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add new command convert to convert SQL migrations to pgroll operations #685

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 18, 2025
Merged

Add new command convert to convert SQL migrations to pgroll operations #685

merged 3 commits into from
Feb 18, 2025

Conversation

kvch
Copy link
Contributor
@kvch kvch commented Feb 14, 2025

This PR adds a skeleton command for converting SQL migration files
to pgroll migration files.

The new command looks like this:

pgroll convert {path-to-sql-file}

All the command does is prints the file name and panics because it is
not implemented.

@kvch kvch requested a review from andrew-farries February 14, 2025 11:45
@kvch kvch marked this pull request as ready for review February 14, 2025 11:46
@kvch kvch requested a review from divyenduz February 14, 2025 11:46
andrew-farries
andrew-farries reviewed Feb 14, 2025
View reviewed changes
Copy link
Collaborator
@andrew-farries andrew-farries left a comment

Choose a reason for hiding this comment

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

We also have the pgroll sql hidden command which takes a SQL string as a parameter and converts its. Do you have an opinion on whether we need one or both of these?

Idea:

pgroll convert could take either a filename, or if none is provided, read from stdin. This would allow piping SQL through convert from some other source.

@kvch
Copy link
Contributor Author
kvch commented Feb 14, 2025

Do you have an opinion on whether we need one or both of these?

I would remove the command sql because it seems unnecessary.

pgroll convert could take either a filename, or if none is provided, read from stdin. This would allow piping SQL through convert from some other source.

Yes, I also considered this. So I like this idea. I was also thinking if the user configures a folder as an input, convert would open all sql files in that folder and tried to translate the migrations into pgroll ops. If the file is not a directory, pgroll opens the file, and translates its contents. I just worry that it might be too confusing with all of these different modes. :D

@kvch kvch mentioned this pull request Feb 17, 2025
Merged
@kvch kvch requested a review from andrew-farries February 18, 2025 10:20
@kvch kvch merged commit 346f7f9 into xataio:main Feb 18, 2025
28 checks passed
@kvch kvch mentioned this pull request Feb 18, 2025
Open
5 tasks
kvch added a commit that referenced this pull request Feb 21, 2025
@kvch
Remove hidden command sql (#687)
136ee5f 
This PR removes a hidden command named `sql` in favour of
`convert`.

The old command will be improved with supporting reading multiple
SQL statements from a single file.

PR for `convert`: #685
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrew-farries andrew-farries andrew-farries approved these changes

@divyenduz divyenduz Awaiting requested review from divyenduz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kvch @andrew-farries
0