Skip to content

Conversation

@brettmc
Copy link
Contributor

@brettmc brettmc commented Dec 23, 2024

Initial work for a 2.x branch:

  • make PHP 8.2 the minimum
  • make 2.x-dev alias
  • bump phpunit dependency
  • run rector over the codebase and implement its recommended 8.2 changes
  • update SPI PackageDependency values

@codecov
Copy link

codecov bot commented Dec 23, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 73.23%. Comparing base (b515b6b) to head (397d818).
Report is 1 commits behind head on 2.x.

Files with missing lines Patch % Lines
src/Context/ZendObserverFiber.php 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #1463      +/-   ##
============================================
- Coverage     73.27%   73.23%   -0.04%     
+ Complexity     2683     2682       -1     
============================================
  Files           387      387              
  Lines          8014     8014              
============================================
- Hits           5872     5869       -3     
- Misses         2142     2145       +3     
Flag Coverage Δ
8.1 ?
8.2 ?
8.3 ?
8.4 73.20% <0.00%> (-0.02%) ⬇️
8.5 73.20% <0.00%> (ø)

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

Files with missing lines Coverage Δ
src/Context/ZendObserverFiber.php 0.00% <0.00%> (ø)

... and 3 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 b515b6b...397d818. Read the comment docs.

@brettmc brettmc marked this pull request as ready for review December 24, 2024 01:31
@brettmc brettmc requested a review from a team as a code owner December 24, 2024 01:31
"branch-alias": {
"dev-main": "1.1.x-dev"
"dev-main": "1.1.x-dev",
"dev-2.x": "2.x-dev"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should consider keeping the API and Context packages on 1.x; bumping these packages to a new major version requires updating every instrumentation package which might slow down adoption of an SDK 2.x release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's reasonable. They will still exist in the 2.x branch (as will proto, semconv), but we can avoid releasing a new version until we do make a breaking change. I think the most likely trigger for this would be removing the registry in favour of SPI (and possibly removing globals initializers at the same time).

namespace OpenTelemetry\SDK\Metrics\Data;

final class Histogram implements DataInterface
final readonly class Histogram implements DataInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW I don't like changing readonly properties to readonly classes automatically as this might result in unnecessary diffs if we decide to change a property back to non-readonly / decide to add a non-readonly property in the future1. I would prefer if we apply this change only to classes that should truly be readonly.

Footnotes

  1. e.g. the DataInterface implementations have mutable $dataPoints in the upstream version of the metrics SDK to support MetricFilter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted the rector changes and disabled this rule

"branch-alias": {
"dev-main": "1.x-dev"
"dev-main": "1.x-dev",
"dev-2.x": "2.x-dev"
Copy link
Contributor

Choose a reason for hiding this comment

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

The version of this package is based on the https://github.com/open-telemetry/semantic-conventions version? -> should stay on 1.x.

"branch-alias": {
"dev-main": "1.x-dev"
"dev-main": "1.x-dev",
"dev-2.x": "2.x-dev"
Copy link
Contributor

Choose a reason for hiding this comment

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

@brettmc
Copy link
Contributor Author

brettmc commented Jan 6, 2025

I've pared this PR back:

  • minimum PHP version is bumped to 8.2 for most components (but, we are not obligated to release a 2.x version of anything until we do have a breaking change)
  • aliases, gitsplit config, github workflow config for 2.x
  • package requirements kept at 1.x, until we do have 2.x version(s)

@brettmc brettmc merged commit 28ccadf into open-telemetry:2.x Jan 9, 2025
9 of 10 checks passed
@brettmc brettmc deleted the 2.x-prep branch January 29, 2025 21:49
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.

3 participants