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

fix: upgraded commitlint to fix warnings #760

Merged
merged 6 commits into from Feb 5, 2020

Conversation

@Ema93sh
Copy link
Contributor

Ema93sh commented Feb 3, 2020

Which problem is this PR solving?

#688

Short description of the changes

Upgraded the @commitlint/cli and @commitlint/config-conventional packages to fix the warnings. The old versions were using core-js@2 and hence the warning messages

@Ema93sh

This comment has been minimized.

Copy link
Contributor Author

Ema93sh commented Feb 3, 2020

I signed it

@Ema93sh

This comment has been minimized.

Copy link
Contributor Author

Ema93sh commented Feb 3, 2020

Looks like the test is failing in CI for unrelated reasons. Will merge this branch with master once it is fixed in master

@dyladan

This comment has been minimized.

Copy link
Contributor

dyladan commented Feb 3, 2020

Looks like the test is failing in CI for unrelated reasons. Will merge this branch with master once it is fixed in master

I'm working on that right now :)

Ema93sh added 3 commits Feb 3, 2020
This reverts commit 4dd8e67.
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Feb 3, 2020

Codecov Report

Merging #760 into master will decrease coverage by 2.59%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master     #760     +/-   ##
=========================================
- Coverage   90.45%   87.86%   -2.6%     
=========================================
  Files         233      148     -85     
  Lines       11501     5750   -5751     
  Branches     1011      476    -535     
=========================================
- Hits        10403     5052   -5351     
+ Misses       1098      698    -400
Impacted Files Coverage Δ
packages/opentelemetry-metrics/src/Meter.ts 97.01% <ø> (ø) ⬆️
...pe-async-hooks/test/AsyncHooksScopeManager.test.ts 100% <0%> (ø) ⬆️
...opentelemetry-core/src/platform/node/timer-util.ts 33.33% <0%> (-65.54%) ⬇️
...ntelemetry-core/test/trace/NoRecordingSpan.test.ts 71.42% <0%> (-27.35%) ⬇️
...pentelemetry-core/src/platform/node/performance.ts 100% <0%> (+10%) ⬆️
...ackages/opentelemetry-core/src/platform/node/id.ts 71.42% <0%> (-16.81%) ⬇️
...try-core/test/trace/fixtures/test-package/index.js 100% <0%> (+17.64%) ⬆️
...elemetry-core/test/trace/spancontext-utils.test.ts 55.55% <0%> (-20.31%) ⬇️
...ckages/opentelemetry-core/test/platform/id.test.ts 77.77% <0%> (+6.34%) ⬆️
packages/opentelemetry-core/test/utils/url.test.ts 50% <0%> (-19.87%) ⬇️
... and 153 more
@mayurkale22

This comment has been minimized.

Copy link
Member

mayurkale22 commented Feb 4, 2020

Thanks for the PR, but warnings are still present in Node 8 build.

@dyladan

This comment has been minimized.

Copy link
Contributor

dyladan commented Feb 4, 2020

Thanks for the PR, but warnings are still present in Node 8 build.

That is not his fault. This is the issue we talked about last week and why we are switching from yarn to npm.

@dyladan
dyladan approved these changes Feb 4, 2020
@dyladan dyladan added the Merge:LGTM label Feb 4, 2020
Ema93sh and others added 2 commits Feb 4, 2020
@mayurkale22 mayurkale22 merged commit 4b764e5 into open-telemetry:master Feb 5, 2020
6 checks passed
6 checks passed
ci/circleci: lint_&_docs Your tests passed on CircleCI!
Details
ci/circleci: node10 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 mayurkale22 authorized
Details
@mayurkale22 mayurkale22 mentioned this pull request Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.