Better handling of multiple migrations #1747

keorn · 2016-07-27T20:25:08Z

coveralls · 2016-07-27T20:51:42Z

Coverage decreased (-0.1%) to 86.768% when pulling 3d38e05 on noop-migrations into 6b1e722 on master.

tomusdrw · 2016-07-28T07:39:44Z

util/src/migration/mod.rs

+	/// Find all needed migrations and arrange them.
+	fn migrations_from(&mut self, version: u32) -> Vec<&mut Box<Migration>> {
+		let mut to_apply: Vec<_> = self.migrations.iter_mut().filter(|m| m.version() > version).collect();
+		to_apply.sort_by_key(|m| m.version());


Why is this needed? We already have a check in add_migration, no?

tomusdrw · 2016-07-28T07:42:13Z

There is also order assumption here: https://github.com/ethcore/parity/blob/3d38e05879f52176e78feeade1ee0ec37c9cb11d/util/src/migration/mod.rs#L233
Probably should be addressed.

tomusdrw · 2016-07-28T09:00:07Z

util/src/migration/mod.rs

@@ -209,7 +209,7 @@ impl Manager {
 		// start with the old db.
 		let old_path_str = try!(old_path.to_str().ok_or(Error::MigrationImpossible));
 		let mut cur_db = try!(Database::open(&db_config, old_path_str).map_err(Error::Custom));
-		for migration in migrations {
+		for migration in migrations.iter_mut() {


https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop

coveralls · 2016-07-28T09:21:45Z

Coverage decreased (-0.1%) to 86.763% when pulling 8a676cb on noop-migrations into 6b1e722 on master.

coveralls · 2016-07-28T09:58:31Z

Coverage decreased (-0.1%) to 86.759% when pulling 3d4ac2c on noop-migrations into 6b1e722 on master.

keorn added 2 commits July 27, 2016 20:54

add migration tests that catch the bug

79c9b1f

make multiple migrations more robust

3d38e05

keorn added the A0-pleasereview 🤓 Pull request needs code review. label Jul 27, 2016

keorn mentioned this pull request Jul 27, 2016

Single DB #1741

Merged

arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 27, 2016

tomusdrw reviewed Jul 28, 2016
View reviewed changes

tomusdrw added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jul 28, 2016

clean up migrations ordering

3ca319b

keorn added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jul 28, 2016

update comment [ci skip]

8a676cb

tomusdrw added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 28, 2016

tomusdrw reviewed Jul 28, 2016
View reviewed changes

remove explicit iter

3d4ac2c

gavofyork merged commit 30ba10e into master Jul 28, 2016

gavofyork deleted the noop-migrations branch July 28, 2016 10:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better handling of multiple migrations #1747

Better handling of multiple migrations #1747

keorn commented Jul 27, 2016

coveralls commented Jul 27, 2016 •

edited

Loading

tomusdrw Jul 28, 2016

tomusdrw commented Jul 28, 2016

tomusdrw Jul 28, 2016 •

edited

Loading

coveralls commented Jul 28, 2016 •

edited

Loading

coveralls commented Jul 28, 2016 •

edited

Loading

Better handling of multiple migrations #1747

Better handling of multiple migrations #1747

Conversation

keorn commented Jul 27, 2016

coveralls commented Jul 27, 2016 • edited Loading

tomusdrw Jul 28, 2016

Choose a reason for hiding this comment

tomusdrw commented Jul 28, 2016

tomusdrw Jul 28, 2016 • edited Loading

Choose a reason for hiding this comment

coveralls commented Jul 28, 2016 • edited Loading

coveralls commented Jul 28, 2016 • edited Loading

coveralls commented Jul 27, 2016 •

edited

Loading

tomusdrw Jul 28, 2016 •

edited

Loading

coveralls commented Jul 28, 2016 •

edited

Loading

coveralls commented Jul 28, 2016 •

edited

Loading