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

Refactor: Consistent package naming and structure #413

Merged
merged 19 commits into from Oct 8, 2019

Conversation

@danielkhan
Copy link
Contributor

commented Oct 8, 2019

Which problem is this PR solving?

closes #408

@codecov-io

This comment has been minimized.

Copy link

commented Oct 8, 2019

Codecov Report

Merging #413 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #413      +/-   ##
==========================================
+ Coverage   95.78%   95.79%   +0.01%     
==========================================
  Files         108      107       -1     
  Lines        5245     5236       -9     
  Branches      434      430       -4     
==========================================
- Hits         5024     5016       -8     
+ Misses        221      220       -1
Impacted Files Coverage Δ
packages/opentelemetry-tracing/src/utility.ts 100% <ø> (ø)
...ges/opentelemetry-exporter-jaeger/src/transform.ts 100% <ø> (ø) ⬆️
...entelemetry-exporter-jaeger/test/transform.test.ts 100% <ø> (ø) ⬆️
...opentelemetry-base/test/resources/resource.test.ts 100% <ø> (ø)
...s/opentelemetry-web/test/StackScopeManager.test.ts 0% <ø> (ø)
packages/opentelemetry-tracing/src/config.ts 100% <ø> (ø)
...ges/opentelemetry-exporter-zipkin/src/transform.ts 100% <ø> (ø) ⬆️
packages/opentelemetry-tracing/src/BasicTracer.ts 100% <ø> (ø)
packages/opentelemetry-web/src/WebTracer.ts 0% <ø> (ø)
...telemetry-plugin-grpc/test/utils/assertionUtils.ts 100% <ø> (ø) ⬆️
... and 39 more
@dyladan
dyladan approved these changes Oct 8, 2019
Copy link
Contributor

left a comment

I like removing the sdk naming everywhere. I agree that the entire project is the sdk so it is redundant to have it on every package name.

danielkhan added 2 commits Oct 8, 2019
@@ -1,4 +1,4 @@
# OpenTelemetry Node SDK

This comment has been minimized.

Copy link
@markwolff

markwolff Oct 8, 2019

Member

Should we still document the fact that this is an SDK at least in the READMEs?

This comment has been minimized.

Copy link
@danielkhan

danielkhan Oct 8, 2019

Author Contributor

It's in the base modules. This package first of all automatic instrumentation of Node.js on top of the SDK.

This comment has been minimized.

Copy link
@danielkhan

danielkhan Oct 8, 2019

Author Contributor

We can still change some wording in the docs as we go. For now, it's important that the package names are right when we publish.

@danielkhan

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2019

Now collides with the newly added postgres package. I'll take care of it.

danielkhan added 2 commits Oct 8, 2019
…-js into rename-packages
@danielkhan danielkhan merged commit a411313 into open-telemetry:master Oct 8, 2019
8 checks passed
8 checks passed
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: node10 Your tests passed on CircleCI!
Details
ci/circleci: node11 Your tests passed on CircleCI!
Details
ci/circleci: node12 Your tests passed on CircleCI!
Details
ci/circleci: node12-browsers Your tests passed on CircleCI!
Details
ci/circleci: node8 Your tests passed on CircleCI!
Details
cla/linuxfoundation danielkhan authorized
Details
@danielkhan danielkhan deleted the danielkhan:rename-packages branch Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.