Skip to content

Add getter for known migrations to runner.#120

Merged
jxs merged 2 commits intorust-db:masterfrom
jakajancar:add-migrations-getter
Jul 14, 2020
Merged

Add getter for known migrations to runner.#120
jxs merged 2 commits intorust-db:masterfrom
jakajancar:add-migrations-getter

Conversation

@jakajancar
Copy link
Contributor

Fixes #117


/// set the target version up to which refinery should migrate, Latest migrates to the latest version available
/// Get the known migrations.
pub fn get_migrations(self) -> Vec<Migration> {
Copy link
Member

Choose a reason for hiding this comment

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

we don't want to consume the Runner, should be &self

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And I'm guessing

pub fn get_migrations(&self) -> &Vec<Migration> {
    &self.migrations
}

is better than

pub fn get_migrations(&self) -> Vec<Migration> {
    self.migrations.clone()
}

(new to Rust :))

Copy link
Member

Choose a reason for hiding this comment

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

yeah, no need to clone, let the caller of the function decide if they want to call clone

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

thanks :)

@jxs jxs merged commit 289397a into rust-db:master Jul 14, 2020
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.

Feature request: Get latest known migration / all known migrations

2 participants