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

Add Granite::Migrator#create_sql and #drop_sql #319

Closed
Jens0512 opened this issue Feb 17, 2019 · 4 comments
Closed

Add Granite::Migrator#create_sql and #drop_sql #319

Jens0512 opened this issue Feb 17, 2019 · 4 comments

Comments

@Jens0512
Copy link
Contributor

Jens0512 commented Feb 17, 2019

tl;dr

Granite::Migrator generates SQL on #drop and #create, and runs it imminently. The possibility of having the migrator export the generated SQL would be nice for use in migration tools like micrate. The generating functionality is already in place, implementing this would just be moving the generating code into create- and drop- _sql methods (names debatable), and using them in place of the current generating code.


I'm using Granite for building a program (i.e. no amber), I'm also quite new to SQL, which is one of the reasons I enjoy Granite so much.

Following the Granite docs on migrating, I stumble upon SQL in the micrate up and down sections, I do not know how Granite interfaces with its tables. Therefore I do not know how to define these migrations, and I take a look at the alternative Granite::Migrator#create and #drop, which both work great, however they seem wholly impractical to use long term, at least compared to micrate.

I hoped I could extract the SQL generated by Granite::Migrator, but seems that's not possible, hence I ask here whether it's a good idea or not to extract the generated code into separate methods. Practical or not it adheres better to the single responsibility principle... Sorry for the spaghetti text.

I'll happily implement this if it's accepted as a good idea.

@Jens0512
Copy link
Contributor Author

Jens0512 commented Feb 18, 2019

#generate_drop_sql may be a better name. (And #generate_create_sql)

@robacarp
Copy link
Member

robacarp commented Feb 21, 2019

@Jens0512 I think this is a fine idea.

create_sql and drop_sql are fine, no need for the generate.

@Jens0512
Copy link
Contributor Author

Great, I can't get the specs working (docker image problems), but I got this done locally. Shall I still just PR?

@robacarp
Copy link
Member

@Jens0512 a PR is welcome, yep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants