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

New relations in the migration engine #612

Merged
merged 1 commit into from Mar 24, 2020

Conversation

tomhoule
Copy link
Contributor

No description provided.

@tomhoule tomhoule added this to the Preview 25 milestone Mar 24, 2020
})
.collect()
Ok((model, table))
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an indentation change.

assert_eq!(row["string"].as_str().unwrap(), "test_string");

Ok(())
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two tests are now obsolete (no implicit defaults anymore).

@tomhoule tomhoule force-pushed the migration-engine/new-relations branch from 906922e to f700954 Compare March 24, 2020 13:54
@tomhoule tomhoule changed the title [WIP] New relations in the migration engine [New relations in the migration engine Mar 24, 2020
@tomhoule tomhoule changed the title [New relations in the migration engine New relations in the migration engine Mar 24, 2020
@tomhoule tomhoule added the tech/engines/migration engine Issue in the Migration Engine label Mar 24, 2020
@tomhoule tomhoule marked this pull request as ready for review March 24, 2020 13:55
@@ -1 +1,2 @@
edition = "2018"
max_width = 120
Copy link
Member

Choose a reason for hiding this comment

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

Yay!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed that because rust-analyzer wasn't respecting the limit in rustfmt.toml (we have two of these, need to figure out why).

auto_increment: {
match f.default_value() {
Some(DefaultValue::Expression(ValueGenerator {
name: _,
Copy link
Member

Choose a reason for hiding this comment

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

Could you use .. here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes most definitely, we can even simplify this whole match to a matches! call. I'll do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplified to matches!(f.default_value(), Some(DefaultValue::Expression(ValueGenerator { generator: ValueGeneratorFn::Autoincrement, .. })))

@tomhoule tomhoule force-pushed the migration-engine/new-relations branch from f700954 to a10ba5f Compare March 24, 2020 14:26
Also two improvements:

- The implicit defaults have been removed
- DateTime @default(now()) now sets a proper database default
@tomhoule tomhoule merged commit 5fb07a0 into virtual-relation-fields Mar 24, 2020
@tomhoule tomhoule deleted the migration-engine/new-relations branch March 24, 2020 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech/engines/migration engine Issue in the Migration Engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants