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

chore: pin API dependency #2213

Closed
wants to merge 2 commits into from
Closed

chore: pin API dependency #2213

wants to merge 2 commits into from

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented May 19, 2021

Which problem is this PR solving?

see #2212 (comment)

Short description of the changes

As long as API is not GA breaking changes are expected therefore API version should be pinned.

As long as API is not GA breaking changes are expected therefore API version should be pinned.
@codecov
Copy link

codecov bot commented May 19, 2021

Codecov Report

Merging #2213 (2142a5d) into main (e379e59) will increase coverage by 1.57%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2213      +/-   ##
==========================================
+ Coverage   92.72%   94.30%   +1.57%     
==========================================
  Files         141       47      -94     
  Lines        5089     1597    -3492     
  Branches     1047      348     -699     
==========================================
- Hits         4719     1506    -3213     
+ Misses        370       91     -279     
Impacted Files Coverage Δ
...s/opentelemetry-core/src/platform/node/sdk-info.ts 0.00% <0.00%> (-100.00%) ⬇️
...pentelemetry-core/src/platform/node/performance.ts 0.00% <0.00%> (-100.00%) ⬇️
...ckages/opentelemetry-exporter-zipkin/src/zipkin.ts 83.92% <0.00%> (-16.08%) ⬇️
packages/opentelemetry-tracing/src/config.ts
...elemetry-tracing/src/export/ConsoleSpanExporter.ts
...elemetry-instrumentation-grpc/src/grpc-js/index.ts
...ry-instrumentation-grpc/src/grpc-js/serverUtils.ts
...opentelemetry-api-metrics/src/NoopMeterProvider.ts
...ource-detector-aws/src/detectors/AwsEc2Detector.ts
...ackages/opentelemetry-metrics/src/CounterMetric.ts
... and 87 more

@Flarna
Copy link
Member Author

Flarna commented May 19, 2021

regarding lint fails see #2204

No idea yet why collector-exporter-grpc tests fail

@blumamir
Copy link
Member

once published to npm, a user installing core and API versions with:
npm install @opentelemetry/node @opentelemetry/api
will get incompatible versions by default as api latest tag in npm points to 0.18.1.

User will have to actively understand that he needs to install a specific version of api like this:
npm install @opentelemetry/node @opentelemetry/api@1.0.0-rc.0 for the setup to work, which might be missed easily and create confusion IMO.

@Flarna
Copy link
Member Author

Flarna commented May 19, 2021

I agree that tags on NPM should be also synced somehow. But that's not directly related to this PR I guess.

@blumamir
Copy link
Member

I agree that tags on NPM should be also synced somehow. But that's not directly related to this PR I guess.

opened an issue in api repo:
open-telemetry/opentelemetry-js-api#72

@Flarna
Copy link
Member Author

Flarna commented May 19, 2021

added a commit to workaround the lint problems. Happy to remove if here are better solutions.

But the question why collector-exporter-grpc tests fail remains.

Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

holding this for now, after talking with maintainers, there is new rc2 which is rc0, should replace the broken rc1. and there will be new plan to handle that

@dyladan
Copy link
Member

dyladan commented May 19, 2021

We are releasing the old rc.0 as rc.2 to fix users who have depended on carat ranges. We will deprecate all 1.0.0-rc.x versions on npm. New release candidates will be released as 0.x.y versions so that they can follow semver.

@Flarna Flarna closed this May 19, 2021
@Flarna
Copy link
Member Author

Flarna commented May 19, 2021

ok, closed this one and the corresponding one in contrib: open-telemetry/opentelemetry-js-contrib#491

@dyladan
Copy link
Member

dyladan commented May 19, 2021

@Flarna Flarna deleted the pin-api branch May 21, 2021 22:04
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.

5 participants