-
Notifications
You must be signed in to change notification settings - Fork 23
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
chore: separate schemas from conditions to allow better combinations/reuse of schemas #591
chore: separate schemas from conditions to allow better combinations/reuse of schemas #591
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## epic-v0.6.x #591 +/- ##
==============================================
Coverage ? 89.00%
==============================================
Files ? 71
Lines ? 6383
Branches ? 165
==============================================
Hits ? 5681
Misses ? 702
Partials ? 0 ☔ View full report in Codecov by Sentry. |
0494f58
to
a193c8b
Compare
bfef228
to
2a51cac
Compare
…etc. Add common single schema that encompasses all possible condition schemas.
…in the correct way.
Include jsonApiConditionSchema in anyConditionSchema.
a193c8b
to
92e5d73
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.
LGTM!
Good TS work here, sir 🫡
jsonApiConditionSchema, | ||
JsonApiConditionType, |
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.
What's the criteria for capitalizing some elements and others not?
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'm unsure if there were criteria, but there was a pre-existing format for earlier conditions where the schema definition was not capitalized, but other types were.
Type of PR:
Required reviews:
What this does:
Currently, condition schemas and condition objects are mixed in the same module. This can generate a circular dependency when trying to reuse schemas for other condition schemas.
Instead, we should separate schemas from conditions so that the schemas can be obtained from a standalone module, increasing their use with other condition schemas.
This change allows us to have a
anyConditionSchema
that includes all condition schemas where any condition can be re-used eg. inCompoundCondition
andSequentialCondition
whose schemas can be recursive.Overall, this PR is simply reshuffling code around and ensuring that the same exports from before are still available.
NOTE that the
schemas
folder is not publicly exported as part of@nucypher/taco
library directly. Instead, schemas are still exported from their original condition module to maintain compatibility.Issues fixed/closed:
Why it's needed:
Notes for reviewers: