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.

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

Added comment reminder to update functions #677

Merged
merged 4 commits into from
Aug 26, 2021
Merged

Conversation

jennuine
Copy link
Collaborator

Summary

Added a TODO comment to update ParamPrivate::TypeToString and ParamPrivate::ValueFromStringImpl functions when new ParamVariant is added.

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

Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
@jennuine jennuine requested a review from azeey August 26, 2021 19:04
@jennuine jennuine self-assigned this Aug 26, 2021
@osrf-triage osrf-triage added this to Inbox in Core development Aug 26, 2021
@github-actions github-actions bot added Gazebo 1️1️ Dependency of Gazebo classic version 11 🏰 citadel Ignition Citadel labels Aug 26, 2021
@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2021

Codecov Report

Merging #677 (39f9fcf) into sdf9 (3a8e069) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             sdf9     #677   +/-   ##
=======================================
  Coverage   86.83%   86.83%           
=======================================
  Files          62       62           
  Lines        9734     9734           
=======================================
  Hits         8453     8453           
  Misses       1281     1281           
Impacted Files Coverage Δ
include/sdf/Param.hh 74.19% <ø> (ø)

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 3a8e069...39f9fcf. Read the comment docs.

@@ -244,6 +244,8 @@ namespace sdf

/// \def ParamVariant
/// \brief Variant type def.
/// TODO(anyone) When a new variant is added, add variant to functions
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would think just a Note would be enough since there's nothing to do in its current state.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Core development automation moved this from Inbox to In review Aug 26, 2021
@jennuine jennuine merged commit aab1512 into sdf9 Aug 26, 2021
Core development automation moved this from In review to Done Aug 26, 2021
@jennuine jennuine deleted the jennuine/param_comment branch August 26, 2021 21:15
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel Gazebo 1️1️ Dependency of Gazebo classic version 11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants