cljoly
Make fn `Migrations::max_schema_version` public.
Addresses #170
fn Migrations::max_schema_version(&self) is a currently private method that returns the number of available migrations wrapped in SchemaVersion.
I would like access to this value in my own code. I'm using Migrations::from_directory() and include_dir! to construct my Migrations, so getting the count myself is non-trivial AFAICT.
My use case is to work out whether any migrations will be run by fn Migrations::to_latest() before running it, and if so to take a backup first.
The library code change is effectively 1 line (1 more to disable a clippy lint on the now-public method, 1 more to improve documentation IMO). Since this adds a new method to the public API I put 2 asserts into an existing test to avoid regressions.
coverage: 95.224%. remained the same when pulling 21dd0d4c12362994fa85620302831faa6b2febcb on fluffysquirrels:pub-max_schema_version into ce9ea7a838b8716eb41211ac5499658fbf6d4bcf on cljoly:master.
coverage: 95.224%. remained the same when pulling cf1bb1711ad0483a5b9927e7be191390ba264f41 on fluffysquirrels:pub-max_schema_version into ce9ea7a838b8716eb41211ac5499658fbf6d4bcf on cljoly:master.
coverage: 95.224%. remained the same when pulling 9421e03cffd650fa9435b2dc9d0ee74f8602353d on fluffysquirrels:pub-max_schema_version into ce9ea7a838b8716eb41211ac5499658fbf6d4bcf on cljoly:master.
@fluffysquirrels thanks for your PR, I've suggested a few improvements. The most important one is adding the same change to the async part of the crate (unfortunately, these things have to be manually synced).
Please feel free to challenge those suggestions.
coverage: 94.97% (-0.3%) from 95.224% when pulling 2edbc029bc39db10e559bd01147fa22c5146e866 on fluffysquirrels:pub-max_schema_version into ce9ea7a838b8716eb41211ac5499658fbf6d4bcf on cljoly:master.
Overall, lgtm, but let's hold merging a little bit: I've posted a comment to further discuss alternative approaches
Overall, lgtm, but let's hold merging a little bit: I've posted a comment to further discuss alternative approaches
Cool. Your changes look good.
I will probably update the PR with a few suggestions from issue #170 so we can see how it looks.
I will probably update the PR with a few suggestions from issue #170 so we can see how it looks.
Just pushed a new version with this.
I also wrapped AsyncMigrations.migrations in an Arc to reduce the cloning work done when running an async method. It was unrelated but a small change and seemed like a quick win.
coverage: 95.029% (+1.0%) from 94.03% when pulling 99b1764217a05c105275fbb2b0220e035b33dbcc on fluffysquirrels:pub-max_schema_version into 313e5325241628112ad0ebac7c07523d97efb8b0 on cljoly:master.
@fluffysquirrels I’ve incorporated the Arc change separately to master and fixed some clippy warnings that came up with the latest rust version. Feel free to squash your commits together when you rebase.
As this is not quite ready yet, I’ve pushed it back to the next stable version 1.4.0 (which could be published as soon as this is ready, potentially).