-
Notifications
You must be signed in to change notification settings - Fork 666
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 <ify1992@yahoo.com>
Co-authored-by: Ifeanyi Ubah <ify1992@yahoo.com>
Co-authored-by: Ifeanyi Ubah <ify1992@yahoo.com>
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
TableConstraintenum. 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:
DisplayandSpannedfor each of the new structsFrom<variant> for TableConstraintfor all variantsThis PR closes issue #2053
Ciao,
Luca