-
-
Notifications
You must be signed in to change notification settings - Fork 314
Cli reader options #1860
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
Cli reader options #1860
Conversation
You are modifying libf3d public API! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1860 +/- ##
==========================================
- Coverage 96.13% 96.11% -0.03%
==========================================
Files 130 130
Lines 11690 11702 +12
==========================================
+ Hits 11238 11247 +9
- Misses 452 455 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Do you need any help or review to move forward @0xfedcafe ? |
Just that upper question about an optional and I’ll push hopefully the final commit for this task with the test |
Hi @0xfedcafe How is it going ? Do you need any help ? |
Hi @0xfedcafe Please do not hesitate to reach out if you need any guidance :) |
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.
It getting there! I've added what is needed for the CI to run the next time you push,
8b9cde1
to
a110778
Compare
Update application/F3DOptionsTools.cxx Co-authored-by: Mathieu Westphal <[email protected]> Update library/src/scene_impl.cxx Co-authored-by: Mathieu Westphal <[email protected]> Apply suggestions from code review Co-authored-by: Mathieu Westphal <[email protected]> merge fix sdk test sdk test & pr review suggestions path check fix fixes
a110778
to
e25b256
Compare
Looks like I forgot this @0xfedcafe , sorry about that. Is this ready to be merged on your side ? |
Hi, yep, I hope I haven’t missed something |
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.
small changes
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.
Some suggestions could not be made:
- doc/user/SUPPORTED_FORMATS.md
- lines 5-40
866fa38
to
0b420b5
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.
Some suggestions could not be made:
- doc/user/SUPPORTED_FORMATS.md
- lines 5-40
0b420b5
to
7141d9e
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.
Some suggestions could not be made:
- doc/user/SUPPORTED_FORMATS.md
- lines 5-40
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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.
Some suggestions could not be made:
- doc/user/SUPPORTED_FORMATS.md
- lines 5-40
Thanks for your contribution @0xfedcafe ! |
Describe your changes
Added a CLI reader option #1735
Issue ticket number and link if any
#1735
Checklist for finalizing the PR
.github/workflows/versions.json
, I have updatedtimestamp
Continuous integration
Please check the checkbox of the CI you want to run, then push again on your branch.