Skip to content

Conversation

@david-luna
Copy link
Contributor

Context

Another PR moving away from the deprecated Detector interface in order to prepare the codebase for a fix for #2320. this time is for GitHub detector.

Not doing all detectors at once:

My reasons to categorize it as a refactor are:

  • SDK handles both interfaces using detectResources from @opentelemetry/resources package. Ref
  • it is unlikely to be used oustide the SDK or auto-instrumentations

Short description of the changes

  • change detect method to be sync

@david-luna david-luna requested a review from a team July 12, 2024 12:55
@github-actions github-actions bot added pkg:resource-detector-github pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found. labels Jul 12, 2024
@codecov
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.40%. Comparing base (dfb2dff) to head (ddbe8bf).
Report is 198 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2336      +/-   ##
==========================================
- Coverage   90.97%   90.40%   -0.58%     
==========================================
  Files         146      149       +3     
  Lines        7492     7359     -133     
  Branches     1502     1527      +25     
==========================================
- Hits         6816     6653     -163     
- Misses        676      706      +30     
Files Coverage Δ
...ce-detector-github/src/detectors/GitHubDetector.ts 100.00% <100.00%> (ø)

... and 66 files with indirect coverage changes

@github-actions
Copy link
Contributor

This package does not have an assigned component owner and is considered unmaintained. As such this package is in feature-freeze and this PR will be closed with 14 days unless a new owner or a sponsor (a member of @open-telemetry/javascript-approvers) for the feature is found. It is the responsibility of the author to find a sponsor for this feature.
Are you familiar with this package? Consider becoming a component owner.

@github-actions
Copy link
Contributor

This issue was closed because no owner or sponsor has been found after 14 days

@github-actions github-actions bot closed this Jul 30, 2024
@pichlermarc
Copy link
Member

I'll sponsor this.

@pichlermarc pichlermarc reopened this Jul 30, 2024
@pichlermarc pichlermarc added has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions and removed pkg-status:unmaintained:autoclose-scheduled labels Jul 30, 2024
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Thanks for updating this 👍

@pichlermarc pichlermarc changed the title refactor(detector-github): change implementation to DetectorSync interface feat(detector-github)!: change implementation to DetectorSync interface Jul 30, 2024
@pichlermarc
Copy link
Member

pichlermarc commented Jul 30, 2024

(changed the title to reflect that this is a breaking change 🙂)

@trentm
Copy link
Contributor

trentm commented Jul 31, 2024

@pichlermarc FYI: David was going to bring up some Qs regarding all the Detector -> DetectorSync PRs in the JS SIG tomorrow. There is some more discussion on #2328

@david-luna
Copy link
Contributor Author

david-luna commented Aug 1, 2024

@pichlermarc FYI: David was going to bring up some Qs regarding all the Detector -> DetectorSync PRs in the JS SIG tomorrow. There is some more discussion on #2328

For the record the SIG we agreed to break for non stable detectors like this one. So I'm merging this one when I can

@david-luna david-luna merged commit d52d421 into open-telemetry:main Aug 1, 2024
@david-luna david-luna deleted the dluna/github-detector-sync branch August 1, 2024 10:14
@dyladan dyladan mentioned this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions pkg:resource-detector-github pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants