Skip to content

Conversation

@jorgenfj
Copy link
Contributor

No description provided.

@jorgenfj jorgenfj requested a review from Andeshog December 20, 2025 19:07
@codecov
Copy link

codecov bot commented Dec 20, 2025

Codecov Report

❌ Patch coverage is 81.48148% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.04%. Comparing base (1120672) to head (89508ea).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/math.cpp 81.48% 2 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #23      +/-   ##
==========================================
- Coverage   98.06%   97.04%   -1.02%     
==========================================
  Files           5        5              
  Lines         413      440      +27     
  Branches       84      103      +19     
==========================================
+ Hits          405      427      +22     
- Misses          4        6       +2     
- Partials        4        7       +3     
Flag Coverage Δ
unittests 97.04% <81.48%> (-1.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
include/vortex/utils/math.hpp 80.00% <ø> (ø)
src/math.cpp 91.66% <81.48%> (-3.40%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

src/math.cpp Outdated
Comment on lines 140 to 141

Eigen::Matrix4d M = Eigen::Matrix4d::Zero();
Copy link
Contributor

Choose a reason for hiding this comment

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

A more appropriate name for this variable, which also follows the naming style, would be nice

src/math.cpp Outdated
Comment on lines 160 to 161

Eigen::Vector4d eigenvector = eigensolver.eigenvectors().col(3);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be const 🤠

Copy link
Contributor

@Andeshog Andeshog left a comment

Choose a reason for hiding this comment

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

Other than that, LGTM

@jorgenfj jorgenfj merged commit 9bb93fa into main Dec 21, 2025
5 checks passed
@jorgenfj jorgenfj deleted the feat/average-quat branch December 21, 2025 13:26
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.

3 participants