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

Replaced common::Time for std::chrono #309

Merged
merged 14 commits into from
Sep 23, 2020
Merged

Replaced common::Time for std::chrono #309

merged 14 commits into from
Sep 23, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Aug 25, 2020

This PR is part of this issue gazebosim/gz-common#61. The idea it's to deprecate the class common::Time in favor of std::chrono.

Related with

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

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde added enhancement New feature or request 🔮 dome Ignition Dome labels Aug 25, 2020
@ahcorde ahcorde self-assigned this Aug 25, 2020
Copy link
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.

This looks good to me. There are some test failure fixes that seem unrelated to the time changes and could go on a separate PR. We also need to wait for upstream releases.

Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Sep 5, 2020
@chapulina chapulina added the beta Targeting beta release of upcoming collection label Sep 8, 2020
@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 17, 2020

@osrf-jenkins run tests

chapulina and others added 4 commits September 17, 2020 17:12
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Sep 23, 2020
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@codecov
Copy link

codecov bot commented Sep 23, 2020

Codecov Report

Merging #309 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #309   +/-   ##
=======================================
  Coverage   77.19%   77.19%           
=======================================
  Files         200      200           
  Lines       10709    10709           
=======================================
  Hits         8267     8267           
  Misses       2442     2442           
Impacted Files Coverage Δ
src/systems/log/LogPlayback.cc 61.13% <ø> (ø)
src/systems/pose_publisher/PosePublisher.cc 94.50% <ø> (ø)
src/systems/air_pressure/AirPressure.cc 75.75% <100.00%> (ø)
src/systems/altimeter/Altimeter.cc 76.47% <100.00%> (ø)
src/systems/imu/Imu.cc 73.68% <100.00%> (ø)
src/systems/logical_camera/LogicalCamera.cc 77.77% <100.00%> (ø)
src/systems/magnetometer/Magnetometer.cc 72.97% <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 7cac3e4...8838a27. Read the comment docs.

@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 23, 2020

is it the bottle updated on osx?

@chapulina
Copy link
Contributor

is it the bottle updated on osx?

I see that osrf/homebrew-simulation#1154 was merged 11 hours ago. Retriggered the homebrew build 🤞

Copy link
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.

CI is happy, good to go! 🚢

@chapulina chapulina merged commit 1117e5d into master Sep 23, 2020
@chapulina chapulina deleted the ahcorde/std_chrono branch September 23, 2020 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection 🔮 dome Ignition Dome enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants