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

Add convenience interface in consumertest that implements all consumers #2878

Merged
merged 2 commits into from
Apr 2, 2021

Conversation

bogdandrutu
Copy link
Member

Signed-off-by: Bogdan Drutu [email protected]

@bogdandrutu bogdandrutu requested a review from a team April 1, 2021 21:12
@bogdandrutu
Copy link
Member Author

/cc @Aneurysm9 @tigrannajaryan @jrcamp please let me know if you think this would help with usability.

@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #2878 (b348f3a) into main (4aeef52) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2878   +/-   ##
=======================================
  Coverage   91.76%   91.77%           
=======================================
  Files         286      286           
  Lines       15100    15104    +4     
=======================================
+ Hits        13857    13861    +4     
  Misses        850      850           
  Partials      393      393           
Impacted Files Coverage Δ
component/componenttest/nop_exporter.go 100.00% <ø> (ø)
component/componenttest/nop_processor.go 100.00% <ø> (ø)
consumer/consumertest/err.go 100.00% <100.00%> (ø)
consumer/consumertest/nop.go 100.00% <100.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 4aeef52...b348f3a. Read the comment docs.

Copy link
Contributor

@jrcamp jrcamp left a comment

Choose a reason for hiding this comment

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

I think it's an improvement.

@Aneurysm9
Copy link
Member

I wonder whether it's worthwhile to combine the nopConsumer and errConsumer since the former seems to be identical to the latter with err == nil.

@bogdandrutu
Copy link
Member Author

@Aneurysm9 is your suggestion to unify only the internal type of also the public API? If it is just the internal we can do it in a followup PR.

@bogdandrutu
Copy link
Member Author

@Aneurysm9 please file an issue with this, I will move forward with the current approach since it is an improvement overall.

@bogdandrutu bogdandrutu merged commit 53832b5 into open-telemetry:main Apr 2, 2021
@bogdandrutu bogdandrutu deleted the consumertst branch April 2, 2021 22:00
pjanotti pushed a commit to pjanotti/opentelemetry-service that referenced this pull request Apr 6, 2021
…rs (open-telemetry#2878)

* Add convenience interface in consumertest that implements all consumers

Signed-off-by: Bogdan Drutu <[email protected]>

* Add changelog entry, fix license

Signed-off-by: Bogdan Drutu <[email protected]>
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
…pen-telemetry#2878)

Bumps [github.com/alecthomas/participle/v2](https://github.com/alecthomas/participle) from 2.0.0-beta.5 to 2.0.0.
- [Release notes](https://github.com/alecthomas/participle/releases)
- [Changelog](https://github.com/alecthomas/participle/blob/master/CHANGES.md)
- [Commits](alecthomas/participle@v2.0.0-beta.5...v2.0.0)

---
updated-dependencies:
- dependency-name: github.com/alecthomas/participle/v2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

3 participants