-
-
Notifications
You must be signed in to change notification settings - Fork 207
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
Use std::optional for options #1617
Conversation
7cf94b0
to
3287f3f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1617 +/- ##
==========================================
+ Coverage 96.82% 96.84% +0.02%
==========================================
Files 108 108
Lines 7643 7668 +25
==========================================
+ Hits 7400 7426 +26
+ Misses 243 242 -1 ☔ View full report in Codecov by Sentry. |
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.
Other than the two comments, that looks very good!
IMO the class method that return all the possible names should be Alternatively, do we actually need the filtered names method in the API in the first place?
|
It was you suggesting to NOT use
This method is needed imo. wdyt @Meakk |
Yes I suggested |
point-size
in two options,point-sprites-size
andpoint-size
scene.animation.time
is initialized to zero, which may have unexepected effect with time range not starting at zero #1354 and add a test for itoptions::isSet
,options::getSetNames
and add testing for itIssues to open: