Skip to content

Revert "publish semantic-convention-utils"#199

Closed
pavan-traceable wants to merge 1 commit intomainfrom
revert-198-publish-semantic-utils
Closed

Revert "publish semantic-convention-utils"#199
pavan-traceable wants to merge 1 commit intomainfrom
revert-198-publish-semantic-utils

Conversation

@pavan-traceable
Copy link
Copy Markdown
Contributor

Reverts #198

@pavan-traceable pavan-traceable requested a review from a team May 29, 2021 08:56
@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2021

Codecov Report

Merging #199 (1602731) into main (b27928f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #199   +/-   ##
=========================================
  Coverage     79.50%   79.50%           
  Complexity     1131     1131           
=========================================
  Files           101      101           
  Lines          4363     4363           
  Branches        405      405           
=========================================
  Hits           3469     3469           
  Misses          704      704           
  Partials        190      190           
Flag Coverage Δ
unit 79.50% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 b27928f...1602731. Read the comment docs.

@github-actions
Copy link
Copy Markdown

Unit Test Results

  65 files  ±0    65 suites  ±0   47s ⏱️ ±0s
315 tests ±0  315 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 1602731. ± Comparison against base commit b27928f.

@skjindal93
Copy link
Copy Markdown
Contributor

We can continue publishing the semantic convention utils, as we discussed, for the downstream consumers to use them

@aaron-steinfeld
Copy link
Copy Markdown
Contributor

We can continue publishing the semantic convention utils, as we discussed, for the downstream consumers to use them

What's the reasoning here? Our architectural plan is to remove all of these utils, so spreading them further is a major issue IMO.

@skjindal93 skjindal93 deleted the revert-198-publish-semantic-utils branch May 31, 2021 05:26
@skjindal93
Copy link
Copy Markdown
Contributor

We can continue publishing the semantic convention utils, as we discussed, for the downstream consumers to use them

What's the reasoning here? Our architectural plan is to remove all of these utils, so spreading them further is a major issue IMO.

Since we are getting rid of the first class fields, how would the consumers read the values from the attributes? I liked the common place of having EnrichedSpanUtils definitely, which is the current way of reading of values from the span

@aaron-steinfeld
Copy link
Copy Markdown
Contributor

mentioned offline, but the attribute reader, not the span utils are meant to be the successor to the first class fields.

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.

4 participants