Skip to content

Suggestion to 469#470

Merged
chapulina merged 1 commit intochapulina/6/mat6_plusfrom
ahcorde/suggestions/469
Jul 21, 2022
Merged

Suggestion to 469#470
chapulina merged 1 commit intochapulina/6/mat6_plusfrom
ahcorde/suggestions/469

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented Jul 21, 2022

Signed-off-by: ahcorde ahcorde@gmail.com

🎉 Suggestions

Summary

Suggestion to 469

  • Matrix6_TEST.py was not added in the CMakeLists.txt
  • Added + operator to Python
  • Added SetValue to Matrix6, this will help with Python, we can't assign a value like this mat(1, 3) = X

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@ahcorde
Suggestion to 469
b685705 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde self-assigned this Jul 21, 2022
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 21, 2022 •
edited
Loading

Codecov Report

Merging #470 (40aadce) into chapulina/6/mat6_plus (728c4a7) will decrease coverage by 0.01%.
The diff coverage is 80.00%.

❗ Current head 40aadce differs from pull request most recent head b685705. Consider uploading reports for the commit b685705 to get more accurate results

@@                    Coverage Diff                    @@
##           chapulina/6/mat6_plus     #470      +/-   ##
=========================================================
- Coverage                  99.68%   99.66%   -0.02%     
=========================================================
  Files                         73       73              
  Lines                       6908     6913       +5     
=========================================================
+ Hits                        6886     6890       +4     
- Misses                        22       23       +1
Impacted Files Coverage Δ
include/ignition/math/Matrix6.hh 99.53% <80.00%> (-0.47%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 728c4a7...b685705. Read the comment docs.

chapulina
chapulina approved these changes Jul 21, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Oh nice catch with the test and ;s! Thanks!

@chapulina chapulina merged commit d049d72 into chapulina/6/mat6_plus Jul 21, 2022
@chapulina chapulina deleted the ahcorde/suggestions/469 branch July 21, 2022 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scpeters scpeters Awaiting requested review from scpeters

@adityapande-1995 adityapande-1995 Awaiting requested review from adityapande-1995

1 more reviewer

@chapulina chapulina chapulina approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ahcorde ahcorde

Labels

None yet

Projects

Core development
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ahcorde @chapulina @osrf-triage