Skip to content

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

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Conversation

0xfedcafe
Copy link
Contributor

Added a CLI reader option #1735

Copy link

github-actions bot commented Jan 3, 2025

You are modifying libf3d public API! ⚠️Please update bindings accordingly⚠️!
You can find them in their respective directories: python, java, webassembly.

Copy link

codecov bot commented Jan 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.89%. Comparing base (4bcd2ac) to head (dbaf7ba).
Report is 59 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1860      +/-   ##
==========================================
+ Coverage   95.75%   95.89%   +0.14%     
==========================================
  Files         123      127       +4     
  Lines       10612    11062     +450     
==========================================
+ Hits        10161    10608     +447     
- Misses        451      454       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mwestphal
Copy link
Member

Do you need any help or review to move forward @0xfedcafe ?

@0xfedcafe
Copy link
Contributor Author

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

@mwestphal
Copy link
Member

Hi @0xfedcafe

How is it going ? Do you need any help ?

@mwestphal mwestphal linked an issue Mar 1, 2025 that may be closed by this pull request
@mwestphal
Copy link
Member

Hi @0xfedcafe

Please do not hesitate to reach out if you need any guidance :)

@mwestphal
Copy link
Member

Hi @0xfedcafe !

Do you need a review or help with CI ?

@0xfedcafe
Copy link
Contributor Author

Hi @0xfedcafe !

Do you need a review or help with CI ?

yes, i think a hint would be very helpful. i can't find why in some tests where the necessary plugins are loaded and the reader is not nullptr it still fails with "unsupported file format"

fixes by mwestphal

Co-authored-by: Mathieu Westphal <mathieu.westphal@gmail.com>
@mwestphal
Copy link
Member

There a small conflict to fix with master, let me know if you need help with that

@mwestphal
Copy link
Member

Hi @0xfedcafe

Do you need help moving forward ?

@mwestphal
Copy link
Member

Hi @0xfedcafe

Do you need help moving forward ?

@0xfedcafe
Copy link
Contributor Author

Hi @0xfedcafe

Do you need help moving forward ?

Hey, yes, I would like to know whether I'm allowed to simply do a disjunction of the parts of error messages in failing test in my test as a regexp or that would be a bad solution? Because I'm unsure why it fails on different platforms with different error messages and I had no access to Linux/Windows so I couldn't test that.

@mwestphal
Copy link
Member

Hi @0xfedcafe
Do you need help moving forward ?

Hey, yes, I would like to know whether I'm allowed to simply do a disjunction of the parts of error messages in failing test in my test as a regexp or that would be a bad solution? Because I'm unsure why it fails on different platforms with different error messages and I had no access to Linux/Windows so I couldn't test that.

I'm confused a bit by your question, lets discuss on discord ? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a CLI option to select reader to use
2 participants