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: add version script to all packages #651

Merged

Conversation

dyladan
Copy link
Member

@dyladan dyladan commented Dec 27, 2019

Which problem is this PR solving?

Short description of the changes

  • Add a script which runs on compile to all packages that adds a version.ts file to the base of every package.

@dyladan
Copy link
Member Author

dyladan commented Dec 27, 2019

Created this PR to simplify the tracer registry PR which is growing quite large

@codecov-io
Copy link

codecov-io commented Dec 27, 2019

Codecov Report

Merging #651 into master will decrease coverage by 0.15%.
The diff coverage is 19.23%.

@@            Coverage Diff             @@
##           master     #651      +/-   ##
==========================================
- Coverage   89.93%   89.77%   -0.16%     
==========================================
  Files         191      213      +22     
  Lines       10182    10103      -79     
  Branches      928      929       +1     
==========================================
- Hits         9157     9070      -87     
- Misses       1025     1033       +8
Impacted Files Coverage Δ
packages/opentelemetry-base/src/version.ts 0% <ø> (ø)
...ges/opentelemetry-scope-async-hooks/src/version.ts 0% <0%> (ø)
packages/opentelemetry-plugin-dns/src/version.ts 0% <0%> (ø)
...kages/opentelemetry-exporter-jaeger/src/version.ts 0% <0%> (ø)
...s/opentelemetry-plugin-mongodb-core/src/version.ts 0% <0%> (ø)
packages/opentelemetry-web/src/version.ts 0% <0%> (ø)
packages/opentelemetry-plugin-http/src/version.ts 0% <0%> (ø)
packages/opentelemetry-plugin-https/src/version.ts 0% <0%> (ø)
packages/opentelemetry-plugin-redis/src/version.ts 0% <0%> (ø)
packages/opentelemetry-metrics/src/version.ts 0% <0%> (ø)
... and 61 more

Copy link
Member

@vmarchaud vmarchaud left a comment

Choose a reason for hiding this comment

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

LGMT, however i believe it would be easier to maintain if we add a scripts folder at the root of the repo

@dyladan
Copy link
Member Author

dyladan commented Dec 30, 2019

@vmarchaud I moved the version script to the root. Definitely agree this makes it easier to maintain.

@dyladan
Copy link
Member Author

dyladan commented Dec 30, 2019

@open-telemetry/javascript-approvers

This is blocking #582 so I would appreciate it if we could get this in fairly quickly. It's a pretty simple change and most of the files are autogenerated.

@dyladan
Copy link
Member Author

dyladan commented Dec 31, 2019

@obecny can you take a look when you get a moment. Since this was initially an issue you made and you wrote the original implementation for core, I feel like I need your feedback before I can merge this.

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.

can you please add version:update in main lerna so it can be run from main folder once for all ?

I was thinking about adding a test to check if all version files were regenerated correctly to be run on circleCi. The main reason for that is that it might be forgotten to regenerate it before commit (happened already).
This could be achieved probably by one bash script, which will check git changes before , run version:update and check if any changes has been done. But that was just the idea and can be done in separate PR.

Other than that lgtm.

@dyladan
Copy link
Member Author

dyladan commented Jan 1, 2020

can you please add version:update in main lerna so it can be run from main folder once for all ?

yep no problem

I was thinking about adding a test to check if all version files were regenerated correctly to be run on circleCi. The main reason for that is that it might be forgotten to regenerate it before commit (happened already).

Is it easier to do that, or just make a preversion script in each package.json? Agree this can be a separate PR

Copy link
Member

@mayurkale22 mayurkale22 left a comment

Choose a reason for hiding this comment

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

lgtm

@mayurkale22 mayurkale22 added the Merge:LGTM This PR is ready to be merged by a Maintainer (has enough valid approvals, successful build, etc.) label Jan 2, 2020
@mayurkale22 mayurkale22 merged commit b158cbe into open-telemetry:master Jan 2, 2020
@Flarna Flarna deleted the version-script branch January 8, 2020 20:31
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge:LGTM This PR is ready to be merged by a Maintainer (has enough valid approvals, successful build, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reading version inside code
7 participants