-
Notifications
You must be signed in to change notification settings - Fork 41
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: enhance attribute value types #1679
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1679 +/- ##
========================================
Coverage 85.73% 85.73%
========================================
Files 607 607
Lines 30828 30831 +3
Branches 8519 8521 +2
========================================
+ Hits 26431 26434 +3
+ Misses 4242 4075 -167
- Partials 155 322 +167
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
codap-v3 Run #5518
Run Properties:
|
Project |
codap-v3
|
Branch Review |
main
|
Run status |
Passed #5518
|
Run duration | 02m 00s |
Commit |
bad513e239: Increment the build number
|
Committer | eireland |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
3
|
View all changes introduced in this branch ↗︎ |
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.
looks good, just the one comment about removing the FIXME
comments.
The reason for the second FIXME was because TS was complaining when the __id__
was first. It said toCanonical
always returned an __id__
so it was unnecessary to set one first. I couldn't figured out why my changes caused this to be a problem so I just moved __id__
to the end. I guess your changes fixed this if TS isn't complaining anymore.
c844dc2
to
b51a36a
Compare
The substantive change is adding
object
toIValueType
and some related types.Many of the other changes are path changes that resulted from moving some definitions into
attribute-types.ts
.