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

change instances of probabilitysampler to traceidratiobasedsampler #179

Conversation

prondubuisi
Copy link
Contributor

@codecov
Copy link

codecov bot commented Sep 19, 2020

Codecov Report

Merging #179 into master will increase coverage by 0.45%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #179      +/-   ##
============================================
+ Coverage     90.52%   90.97%   +0.45%     
  Complexity      285      285              
============================================
  Files            35       35              
  Lines           665      665              
============================================
+ Hits            602      605       +3     
+ Misses           63       60       -3     
Impacted Files Coverage Δ Complexity Δ
sdk/Trace/Sampler/TraceIdRatioBasedSampler.php 90.90% <100.00%> (ø) 6.00 <0.00> (?)
sdk/Metrics/ValueRecorder.php 88.00% <0.00%> (+4.00%) 10.00% <0.00%> (ø%)
sdk/Metrics/Counter.php 84.61% <0.00%> (+7.69%) 5.00% <0.00%> (ø%)
sdk/Metrics/UpDownCounter.php 66.66% <0.00%> (+8.33%) 5.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f158ad...1f1a1d4. Read the comment docs.

@beniamin
Copy link
Contributor

Should be ready to merge after you sign with CLA.

@prondubuisi
Copy link
Contributor Author

Should be ready to merge after you sign with CLA.

Hello @beniamin I am not seeing any prompt to do the signing as was the case in my first PR, would signing this using git commit -S -m work?

@beniamin
Copy link
Contributor

I think all you need to do is to force a new commit, I saw that in your previous PR the CLA check has passed. Maybe now it's just stucked in "waiting for stauts to be reported."

@prondubuisi prondubuisi force-pushed the refactor/probalitysampler-to-newspecification branch from 0ec1674 to 1f1a1d4 Compare September 20, 2020 05:19
@prondubuisi
Copy link
Contributor Author

I think all you need to do is to force a new commit, I saw that in your previous PR the CLA check has passed. Maybe now it's just stucked in "waiting for stauts to be reported."

This works. Thanks. Can you help me look into my comments here #174

@bobstrecansky bobstrecansky merged commit 871fefb into open-telemetry:master Sep 21, 2020
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.

Naming Updates
3 participants