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

migrate Clock classes to API #1281

Merged
merged 6 commits into from
Apr 29, 2024
Merged

Conversation

brettmc
Copy link
Collaborator

@brettmc brettmc commented Apr 19, 2024

  • move Clock* + Util into API
  • add a deprecated wrapper for ClockFactory and Util
  • remove unused StopWatch* classes

Fixes: #1280

- move Clock* + Util into API
- add a deprecated wrapper for ClockFactory and Util
- remove unused StopWatch* classes
@brettmc brettmc marked this pull request as ready for review April 22, 2024 06:33
@brettmc brettmc requested a review from a team April 22, 2024 06:33
Copy link
Contributor

@ChrisLightfootWild ChrisLightfootWild left a comment

Choose a reason for hiding this comment

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

Amazing, thanks for this! ⏱️

src/API/Common/Time/ClockFactory.php Outdated Show resolved Hide resolved
src/API/Common/Time/Util.php Outdated Show resolved Hide resolved
src/API/Common/Time/ClockInterface.php Outdated Show resolved Hide resolved
brettmc added 2 commits April 24, 2024 10:37
It's not really a factory, and really only provides access to a system clock. Create a
Clock class which is more clear in its purpose.
per review feedback, it's only used in Zipkin. Removed some unused consts from ClockInterface
src/API/Common/Time/Clock.php Outdated Show resolved Hide resolved
src/API/Common/Time/Clock.php Outdated Show resolved Hide resolved
Copy link

codecov bot commented Apr 27, 2024

Codecov Report

Attention: Patch coverage is 82.60870% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 73.97%. Comparing base (cea2d7c) to head (477b845).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1281      +/-   ##
============================================
- Coverage     74.31%   73.97%   -0.34%     
+ Complexity     2391     2365      -26     
============================================
  Files           349      347       -2     
  Lines          7124     7071      -53     
============================================
- Hits           5294     5231      -63     
- Misses         1830     1840      +10     
Flag Coverage Δ
8.1 73.97% <82.60%> (-0.34%) ⬇️
8.2 73.97% <82.60%> (-0.34%) ⬇️
8.3 73.97% <82.60%> (-0.34%) ⬇️
8.4 72.84% <82.60%> (-0.35%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/API/Common/Time/Clock.php 100.00% <100.00%> (ø)
src/API/Common/Time/SystemClock.php 64.28% <ø> (ø)
src/API/Common/Time/TestClock.php 0.00% <ø> (ø)
src/Contrib/Zipkin/SpanConverter.php 96.45% <100.00%> (+0.05%) ⬆️
src/SDK/Logs/LogRecordProcessorFactory.php 100.00% <100.00%> (ø)
src/SDK/Logs/Processor/BatchLogRecordProcessor.php 100.00% <ø> (ø)
src/SDK/Metrics/Meter.php 76.11% <ø> (ø)
src/SDK/Metrics/MeterProvider.php 85.71% <ø> (ø)
src/SDK/Metrics/MetricRegistry/MetricRegistry.php 77.10% <ø> (ø)
...talenessHandler/DelayedStalenessHandlerFactory.php 100.00% <ø> (ø)
... and 8 more

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cea2d7c...477b845. Read the comment docs.

@brettmc brettmc merged commit fc161a8 into open-telemetry:main Apr 29, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move ClockInterface / ClockFactory from SDK to API
6 participants