Skip to content

Deprecate math::clock alias#600

Merged
mjcarroll merged 3 commits intomainfrom
scpeters/deprecate_math_clock_alias
Jul 2, 2024
Merged

Deprecate math::clock alias#600
mjcarroll merged 3 commits intomainfrom
scpeters/deprecate_math_clock_alias

Conversation

@scpeters
Copy link
Copy Markdown
Member

@scpeters scpeters commented Jul 1, 2024

🎉 New feature

Closes #436.

Summary

Recommend using std::chrono::steady_clock directly instead of this alias. This should already have been removed from downstream gz-* packages.

Test it

Compile Ionic workspace with this branch and watch for deprecation warnings.

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.

Recommend using std::chrono::steady_clock directly
instead of this alias.

Fixes #436.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@github-actions github-actions Bot added the 🏛️ ionic Gazebo Ionic label Jul 1, 2024
Comment thread include/gz/math/Stopwatch.hh Outdated
using clock = std::chrono::steady_clock;
// This alias is now deprecated; please use std::chrono::steady_clock
// directly instead.
using clock GZ_DEPRECATED(8) = std::chrono::steady_clock;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

windows CI doesn't like this

Stopwatch.hh(33,17): error C2062: type 'int' unexpected

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

trying [[deprecated]] based on https://stackoverflow.com/a/50882131

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Comment thread src/Stopwatch_TEST.cc
EXPECT_LT(watch.StopTime(), watch.StartTime());
EXPECT_NE(math::clock::duration::zero(), watch.ElapsedRunTime());
EXPECT_EQ(math::clock::duration::zero(), watch.ElapsedStopTime());
EXPECT_NE(steady_clock::duration::zero(), watch.ElapsedRunTime());
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

windows failed on this line; I think it's flaky

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.97%. Comparing base (c32e96e) to head (9d757ac).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #600   +/-   ##
=======================================
  Coverage   95.97%   95.97%           
=======================================
  Files         147      147           
  Lines       10122    10123    +1     
=======================================
+ Hits         9715     9716    +1     
  Misses        407      407           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mjcarroll mjcarroll merged commit 15bfa5e into main Jul 2, 2024
@mjcarroll mjcarroll deleted the scpeters/deprecate_math_clock_alias branch July 2, 2024 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏛️ ionic Gazebo Ionic

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants