Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Added capsule mesh #155

Merged
merged 8 commits into from
Jan 22, 2021
Merged

Added capsule mesh #155

merged 8 commits into from
Jan 22, 2021

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Jan 21, 2021

Related with this issue #140

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

@ahcorde
Added capsule mesh
26c6f65 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from mjcarroll as a code owner January 21, 2021 14:28
@ahcorde ahcorde self-assigned this Jan 21, 2021
@github-actions github-actions bot added 🏢 edifice Ignition Edifice 🏰 citadel Ignition Citadel 📜 blueprint Ignition Blueprint 🔮 dome Ignition Dome labels Jan 21, 2021
@ahcorde ahcorde mentioned this pull request Jan 21, 2021
Merged
@codecov
Copy link

codecov bot commented Jan 21, 2021 •
edited
Loading

Codecov Report

Merging #155 (1509754) into ign-common3 (b68a962) will increase coverage by 0.23%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-common3     #155      +/-   ##
===============================================
+ Coverage        75.53%   75.76%   +0.23%     
===============================================
  Files               69       69              
  Lines             9623     9715      +92     
===============================================
+ Hits              7269     7361      +92     
  Misses            2354     2354
Impacted Files Coverage Δ
graphics/src/MeshManager.cc 85.69% <100.00%> (+2.11%) ⬆️

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 b68a962...1509754. Read the comment docs.

@ahcorde
Added capsule test
7ad6e51 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
removed code
ad24eab 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
fixed capsule test variable names
97707fe 
Signed-off-by: ahcorde <ahcorde@gmail.com>
mjcarroll
mjcarroll reviewed Jan 21, 2021
View reviewed changes
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

Largely the same comments as #154

graphics/include/ignition/common/MeshManager.hh Outdated Show resolved Hide resolved
graphics/src/MeshManager.cc Show resolved Hide resolved
graphics/src/MeshManager.cc Show resolved Hide resolved
graphics/src/MeshManager.cc Outdated Show resolved Hide resolved
graphics/src/MeshManager.cc Outdated Show resolved Hide resolved
graphics/src/MeshManager.cc Outdated Show resolved Hide resolved
@ahcorde
Added feedback
ea3c3ab 
Signed-off-by: ahcorde <ahcorde@gmail.com>
mjcarroll
mjcarroll reviewed Jan 21, 2021
View reviewed changes
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

Just a few style nits

graphics/src/MeshManager.cc Outdated Show resolved Hide resolved
graphics/src/MeshManager.cc Outdated Show resolved Hide resolved
graphics/src/MeshManager.cc Outdated Show resolved Hide resolved
graphics/src/MeshManager.cc Outdated Show resolved Hide resolved
graphics/src/MeshManager.cc Outdated Show resolved Hide resolved
@ahcorde
Fixed style
f6f7228 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from mjcarroll January 22, 2021 07:02
@ahcorde
make windows happy
b4792a3 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from mjcarroll January 22, 2021 15:03
mjcarroll
mjcarroll approved these changes Jan 22, 2021
View reviewed changes
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

LGTM once windows comes back happy.

Thanks for iterating so quickly!

@ahcorde
Copy link
Contributor Author

ahcorde commented Jan 22, 2021 •
edited
Loading

This failing test is not part of this PR

  • VideoEncoderTest.Exists

@ahcorde ahcorde merged commit 248d1e9 into ign-common3 Jan 22, 2021
@ahcorde ahcorde deleted the ahcorde/mesh/capsule branch January 22, 2021 19:44
@mjcarroll mjcarroll mentioned this pull request Feb 3, 2021
Closed
@chapulina chapulina mentioned this pull request Mar 22, 2021
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjcarroll mjcarroll mjcarroll approved these changes

Assignees

@ahcorde ahcorde

Labels
📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ahcorde @mjcarroll