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

Using dbmigrate programmatically -> why is it not a lib/why is functionality not in dbmigrate-lib #26

Open
ghost opened this issue Dec 22, 2017 · 16 comments

Comments

@ghost
Copy link

ghost commented Dec 22, 2017

Hello!

Consider myself a noobie, please.

I am (successfully) using your awesome migration tool (thanks for that!) programmatically in an application of mine.

To get it working I had a look at your implementation of dbmigrate (as suggested in the readme) and basically more or less re-implemented the same macro and functions myself.

dbmigrate is not a library (correct me if I am wrong and simply too dumb to get the crate somehow).

That leaves me with 2 questions/proposals:

Can it be done that functionality (e.g. migrate macro and up etc. functions) be moved to dbmigrate-lib?

If not (I would be curious as to the why then, actually really curious why they were done in the dbmigrate - as said, consider myself a noobie!), could dbmigrate be also made a lib?

Thanks again for your time and efforts!

@Keats
Copy link
Owner

Keats commented Dec 22, 2017

Hey
I agree that some of the functions in dbmigrate should probably be in dbmigrate-lib. It's mostly due to the fact that dbmigrate started as a binary only and some parts of it moved to a lib later on but I didn't move everything. I'll do that over the weekend, should be a simple case of copy/paste and publishing a new release.

@Keats
Copy link
Owner

Keats commented Dec 25, 2017

I just saw the reason why I didn't move the code to the lib, it has lots of println! in the middle which you don't want when using as a library. It also uses a print crate for it which you want even less as a lib

@ghost
Copy link
Author

ghost commented Dec 27, 2017

The question is why that is tied to the print crate in the first place. One could simply return Result and let the user of the lib handle it appropriately. (For dbmigrate that would mean using the print crate)

@Keats
Copy link
Owner

Keats commented Dec 27, 2017

I guess the migrate macro could be turned into a function into dbmigrate-lib but the others cannot just be changed to a Result that easily. For example, up and down will need to return a Vec<Result<_>> since it can run many migrations in one go

@sivakov512
Copy link

Any news here? I want to use dbmigrate programmatically but it requires copy\paste from dbmigrate crate into my project.

@Keats
Copy link
Owner

Keats commented Aug 25, 2019

I don't use that crate myself but I will merge a good implementation of that issue

@sivakov512
Copy link

sivakov512 commented Aug 25, 2019

Why you stop using it?

but I will merge a good implementation of that issue

You mean that I can contribute with PR?

@Keats
Copy link
Owner

Keats commented Aug 25, 2019

Why you stop using it?

I don't do anything database related right now with Rust.

You mean that I can contribute with PR?

Yep!

@sivakov512
Copy link

Oh, it requires a lot of refactoring, if I understand correctly?

@sivakov512
Copy link

How can we organize this work so that I don’t do a very large pull request for review at once?

@ghost
Copy link

ghost commented Aug 26, 2019

I wrote some code a while back, didn't come around to get it ready for a PR yet. Tested with SQLite and Postgres, but not with MySQL: https://github.com/FSMaxB/dbmigrate/tree/simplify-migration-struct

@ghost
Copy link

ghost commented Aug 26, 2019

This relates to #38 which makes it easier to use the library programmatically since you don't need to check for None-Optionals everywhere.

@Keats
Copy link
Owner

Keats commented Aug 26, 2019

Yep #38 is a great way to start. After that, the changes should not be THAT big.

@sivakov512
Copy link

I think that we should start with move code with current implementation into lib as much as we can. Right?
Is not sure #38 is relevant here. Could you please explain it?

@Keats
Copy link
Owner

Keats commented Aug 27, 2019

#38 is just simplifying the code, not strictly required but it makes it easier to think about.

@sivakov512
Copy link

sivakov512 commented Aug 27, 2019

You think that with this refactoring we should not only move code into lib?

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

No branches or pull requests

2 participants