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

Deprecated Processor interface and moved to own package #2326

Conversation

MovieStoreGuy
Copy link

Helping contribute to #2273

Help speed up development of metrics SDK for Go
@MovieStoreGuy MovieStoreGuy force-pushed the smarciniak/migrate-sdk-export-processor branch from 7196452 to 497ecba Compare October 27, 2021 04:16
@codecov
Copy link

codecov bot commented Oct 27, 2021

Codecov Report

Merging #2326 (79df271) into main (0f0bd26) will decrease coverage by 0.0%.
The diff coverage is n/a.

❗ Current head 79df271 differs from pull request most recent head e64c386. Consider uploading reports for the commit e64c386 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2326     +/-   ##
=======================================
- Coverage   73.6%   73.6%   -0.1%     
=======================================
  Files        175     175             
  Lines      12409   12409             
=======================================
- Hits        9137    9134      -3     
- Misses      3039    3041      +2     
- Partials     233     234      +1     
Impacted Files Coverage Δ
sdk/export/metric/metric.go 0.0% <ø> (ø)
...s/otlp/otlptrace/internal/connection/connection.go 14.6% <0.0%> (-1.6%) ⬇️

Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

Overall this change set looks good. Only minor issues to address.

CHANGELOG.md Outdated Show resolved Hide resolved
sdk/export/metric/metric.go Outdated Show resolved Hide resolved
@@ -401,7 +401,6 @@ github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/itchyny/go-flags v1.5.0 h1:Z5q2ist2sfDjDlExVPBrMqlsEDxDR2h4zuOElB0OEYI=
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this change? It looks like it is reverting #2316.

I'm guessing the tooling automatically did this and we should look into not having it clear this.

CHANGELOG.md Outdated Show resolved Hide resolved
MovieStoreGuy and others added 3 commits November 1, 2021 17:33
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@MovieStoreGuy
Copy link
Author

@jmacd I've notice and issue is doing via smaller deprecations means doing an odd alias back to package I am trying to deprecate.

As much as I thought it made sense to do smaller cuts and deprecations, I think it has to be done within the one change in order to avoid situations are trying to alias the would be deprecated package in the packages the types are moving to.

I am gonna close off the smaller PRs and do it in one clean sweep

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.

None yet

3 participants