-
Couldn't load subscription status.
- Fork 537
feat: add multiple constraints at once #3879
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
feat: add multiple constraints at once #3879
Conversation
|
ACTION NEEDED delta-rs follows the Conventional Commits specification for release automation. The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3879 +/- ##
==========================================
+ Coverage 73.70% 73.73% +0.03%
==========================================
Files 151 151
Lines 39260 39347 +87
Branches 39260 39347 +87
==========================================
+ Hits 28937 29013 +76
- Misses 9015 9023 +8
- Partials 1308 1311 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a6bc2c7 to
252b51e
Compare
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.
Good stuff! Just one comment for small improvement :)
| } | ||
|
|
||
| /// Specify the constraint to be added | ||
| pub fn with_constraint<S: Into<String>, E: Into<Expression>>( |
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.
Maybe one small nit, i think we can keep this where this just inserts in the hashmap and then we also have with_constraints which is the hashmap version. This way we don't entirely break the signature
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.
Ah yes that makes sense! Do we want to restrict using both with_constraint and with_constraints at the same time? At first I had with_multiple_constraints that extend the hashmap just in case someone used both builder functions
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.
Yeah you could have them both extend it, always
252b51e to
a5a9619
Compare
Signed-off-by: JustinRush80 <[email protected]>
…expression is different Signed-off-by: JustinRush80 <[email protected]>
Signed-off-by: JustinRush80 <[email protected]>
Signed-off-by: JustinRush80 <[email protected]>
Signed-off-by: JustinRush80 <[email protected]>
Signed-off-by: JustinRush80 <[email protected]>
ee317e5 to
0458633
Compare
Description
This PR allow users to add multiple constraints at once using an HashMap.
Related Issue(s)
Documentation