-
Notifications
You must be signed in to change notification settings - Fork 654
Moved constraint variant outside of TableConstraint
enum
#2054
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
Conversation
@@ -0,0 +1,67 @@ | |||
// Licensed to the Apache Software Foundation (ASF) under one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use a single file table_constraints.rs
to contain the new structs, vs having one struct per file. So that we don't end up accumulating a lot of small files which would be the other extreme of where we are today
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok - should the table_constraints.rs file contain the TableConstraint
enum itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit 258cc8c
Co-authored-by: Ifeanyi Ubah <[email protected]>
Co-authored-by: Ifeanyi Ubah <[email protected]>
Co-authored-by: Ifeanyi Ubah <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @LucaCappelletti94!
As per title, this PR moved all of the struct variants out of the
TableConstraint
enum. This is done to allow for implementing traits in dependent crates which only apply to some of the variants and not all of them.Furthermore it:
Display
andSpanned
for each of the new structsFrom<variant> for TableConstraint
for all variantsThis PR closes issue #2053
Ciao,
Luca