Skip to content

Conversation

@cuongdv-20
Copy link

@cuongdv-20 cuongdv-20 commented Jan 3, 2026

Describe your changes

Support to get list of time step in vtkF3DAssimpImporter::GetTemporalInformation

Issue ticket number and link if any

#2735

Checklist for finalizing the PR

  • I have performed a self-review of my code
  • I have added tests for new features and bugfixes
  • I have added documentation for new features
  • If it is a modifying the libf3d API, I have updated bindings
  • If it is a modifying the .github/workflows/versions.json, I have updated docker_timestamp

Continuous integration

Please write a comment to run CI, eg: \ci fast.
See here for more info.

@cuongdv-20
Copy link
Author

Hi @mwestphal, I updated new patch, could you help me review it?

@mwestphal
Copy link
Member

\ci fast

Copy link
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, a small suggestion

@cuongdv-20 cuongdv-20 force-pushed the support_recovering_keyframes_timesteps_vtkF3DAssimpImporter branch from 96c9b61 to e3e1edc Compare January 8, 2026 12:36
@mwestphal
Copy link
Member

\ci full

Copy link
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small changes

mwestphal

This comment was marked as outdated.

@cuongdv-20 cuongdv-20 requested a review from mwestphal January 8, 2026 15:05
@mwestphal
Copy link
Member

If CI is clean Ill merge :)

@codecov
Copy link

codecov bot commented Jan 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.64%. Comparing base (57849e0) to head (889c492).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2770   +/-   ##
=======================================
  Coverage   96.64%   96.64%           
=======================================
  Files         142      142           
  Lines       12933    12949   +16     
=======================================
+ Hits        12499    12515   +16     
  Misses        434      434           

☔ 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

Looks like some lines are not covered, do youb think you can try testing with other files to cover these lines:

https://app.codecov.io/gh/f3d-app/f3d/pull/2770?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=f3d-app

@mwestphal
Copy link
Member

there is no need to merge with master

@cuongdv-20
Copy link
Author

Looks like some lines are not covered, do youb think you can try testing with other files to cover these lines:

https://app.codecov.io/gh/f3d-app/f3d/pull/2770?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=f3d-app

There are no files in the testing/data directory containing mNumMeshChannels and mNumMorphMeshChannels > 0. It seems I need to create a new file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants