Skip to content

Remove Cylinder::SetLength const method#259

Merged
scpeters merged 2 commits intoign-math6from
ahcorde/cylinder/clean_header
Oct 4, 2021
Merged

Remove Cylinder::SetLength const method#259
scpeters merged 2 commits intoign-math6from
ahcorde/cylinder/clean_header

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented Oct 4, 2021

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

🦟 Bug fix

Summary

Remove SetLength() const method. I think this method does not make sense. It's imposible to set the Lenght in a const method.

There is no implementation for this method.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

@ahcorde
Removed SetLength const method
cc3ad0c 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from azeey October 4, 2021 20:43
@ahcorde ahcorde self-assigned this Oct 4, 2021
@ahcorde ahcorde requested a review from scpeters as a code owner October 4, 2021 20:43
@github-actions github-actions bot added Gazebo 1️1️ Dependency of Gazebo classic version 11 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome labels Oct 4, 2021
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 4, 2021 •
edited
Loading

Codecov Report

Merging #259 (7ad391a) into ign-math6 (4c1259a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           ign-math6     #259   +/-   ##
==========================================
  Coverage      99.41%   99.41%           
==========================================
  Files             67       67           
  Lines           6347     6347           
==========================================
  Hits            6310     6310           
  Misses            37       37
Impacted Files Coverage Δ
include/ignition/math/Cylinder.hh 100.00% <ø> (ø)

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 4c1259a...7ad391a. Read the comment docs.

@chapulina
Copy link
Copy Markdown
Contributor

chapulina commented Oct 4, 2021

Interesting that the ABI checker didn't complain. Is it because it's not implemented?

@azeey
Copy link
Copy Markdown
Contributor

azeey commented Oct 4, 2021

Interesting that the ABI checker didn't complain. Is it because it's not implemented?

I think it's because Cylinder is a template.

@scpeters scpeters merged commit 23a24d7 into ign-math6 Oct 4, 2021
@scpeters scpeters deleted the ahcorde/cylinder/clean_header branch October 4, 2021 23:17
@scpeters scpeters changed the title Removed SetLength const method Remove Cylinder::SetLength const method Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@azeey azeey azeey approved these changes

@scpeters scpeters scpeters approved these changes

Assignees

@ahcorde ahcorde

Labels

🏰 citadel Ignition Citadel 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress Gazebo 1️1️ Dependency of Gazebo classic version 11 hacktoberfest-accepted

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ahcorde @chapulina @azeey @scpeters @j-rivero @osrf-triage