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.

Sign up for GitHub

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

Jump to bottom

remove generated jsonschema code #8620

Merged
merged 3 commits into from Nov 16, 2023
Merged

remove generated jsonschema code #8620

merged 3 commits into from Nov 16, 2023

Conversation

codeboten
Copy link
Contributor

This code will live in the opentelemetry-go-contrib repo in the future.

This depends on open-telemetry/opentelemetry-go-contrib#4376

@codeboten codeboten mentioned this pull request Oct 4, 2023
Closed
2 tasks
@codeboten codeboten force-pushed the codeboten/rm-generated-jsonschema-code branch 2 times, most recently from 3c36eb6 to 10b1f79 Compare October 10, 2023 19:34
@codeboten codeboten marked this pull request as ready for review October 10, 2023 19:34
@codeboten codeboten requested a review from a team as a code owner October 10, 2023 19:34
@codeboten codeboten force-pushed the codeboten/rm-generated-jsonschema-code branch from 10b1f79 to c321970 Compare October 10, 2023 19:36
@codeboten codeboten changed the title [wip] remove generated jsonschema code remove generated jsonschema code Oct 10, 2023
@codecov
Copy link

codecov bot commented Oct 10, 2023 •
edited

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (f96d155) 90.82% compared to head (0c9b2cf) 91.60%.
Report is 6 commits behind head on main.

Files Patch % Lines
internal/testutil/testutil.go 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8620      +/-   ##
==========================================
+ Coverage   90.82%   91.60%   +0.77%     
==========================================
  Files         318      317       -1     
  Lines       17208    17013     -195     
==========================================
- Hits        15630    15585      -45     
+ Misses       1284     1134     -150     
  Partials      294      294

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codeboten codeboten mentioned this pull request Oct 11, 2023
Closed
@codeboten codeboten force-pushed the codeboten/rm-generated-jsonschema-code branch from 7a9d332 to 1ab171a Compare October 11, 2023 17:25
@mx-psi mx-psi mentioned this pull request Oct 16, 2023
Closed
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 26, 2023
@codeboten codeboten removed the Stale label Oct 28, 2023
@codeboten codeboten mentioned this pull request Nov 1, 2023
Closed
@github-actions GitHub Actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Nov 11, 2023
@codeboten codeboten force-pushed the codeboten/rm-generated-jsonschema-code branch from 1ab171a to bfa54db Compare November 11, 2023 13:14
@codeboten codeboten removed the Stale label Nov 11, 2023
@codeboten codeboten force-pushed the codeboten/rm-generated-jsonschema-code branch from bfa54db to 413faaa Compare November 11, 2023 13:17
@codeboten codeboten mentioned this pull request Nov 11, 2023
Merged
@mx-psi
Copy link
Member

mx-psi commented Nov 13, 2023

open-telemetry/opentelemetry-go-contrib#4376 has been released on
https://github.com/open-telemetry/opentelemetry-go-contrib/releases/tag/v1.21.0, this can now move forward 🚀

@codeboten codeboten force-pushed the codeboten/rm-generated-jsonschema-code branch from d6ccfa5 to a69f7e7 Compare November 14, 2023 20:23
@codeboten
Copy link
Contributor Author

@mx-psi updated the PR with v0.1.0

mx-psi
mx-psi reviewed Nov 15, 2023
View reviewed changes
exporter/otlpexporter/go.mod Outdated Show resolved Hide resolved
exporter/otlphttpexporter/go.mod Outdated Show resolved Hide resolved
extension/zpagesextension/go.mod Outdated Show resolved Hide resolved
receiver/otlpreceiver/go.mod Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Accidental revert of #8795?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, thanks for catching. fixed

service/telemetry/generated_config.go Outdated Show resolved Hide resolved
service/telemetry/config_test.go Outdated
@@ -67,177 +63,178 @@ func TestLoadConfig(t *testing.T) {
}
}

// TODO: review where unmarshal tests should live
Copy link
Member

Choose a reason for hiding this comment

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

I would just remove the code or keep it as is, leaving it commented does not seem useful to me (we always have git history). If we remove it, we can file an issue to track this.

Copy link
Member

Choose a reason for hiding this comment

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

I'd say let's remove this and file an issue? That seems like the easiest option

Alex Boten added 2 commits November 15, 2023 14:51
[wip] remove generated jsonschema code
1b309ee 
This code lives in the opentelemetry-go-contrib repo now.

Signed-off-by: Alex Boten <aboten@lightstep.com>
clean up merge issues
2d8ada2 
Signed-off-by: Alex Boten <aboten@lightstep.com>
@codeboten codeboten force-pushed the codeboten/rm-generated-jsonschema-code branch from 8bb123b to 2d8ada2 Compare November 15, 2023 22:56
mx-psi
mx-psi approved these changes Nov 16, 2023
View reviewed changes
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Approving but please see the config_test.go comment

service/telemetry/config_test.go Outdated
@@ -67,177 +63,178 @@ func TestLoadConfig(t *testing.T) {
}
}

// TODO: review where unmarshal tests should live
Copy link
Member

Choose a reason for hiding this comment

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

I'd say let's remove this and file an issue? That seems like the easiest option

cleanup disabled tests
0c9b2cf 
Signed-off-by: Alex Boten <aboten@lightstep.com>
@codeboten codeboten merged commit a82663e into open-telemetry:main Nov 16, 2023
31 of 32 checks passed
@codeboten codeboten deleted the codeboten/rm-generated-jsonschema-code branch November 16, 2023 19:01
@github-actions github-actions bot added this to the next release milestone Nov 16, 2023
@dmitryax dmitryax mentioned this pull request Dec 12, 2023
Merged
codeboten pushed a commit that referenced this pull request Dec 12, 2023
@dmitryax
[chore] Remove jsonschema artifacts (#9086)
37f94af 
Should've been removed as part of
#8620
sokoide pushed a commit to sokoide/opentelemetry-collector that referenced this pull request Dec 18, 2023
@dmitryax @sokoide
[chore] Remove jsonschema artifacts (open-telemetry#9086)
8450e2e 
Should've been removed as part of
open-telemetry#8620
@atoulme atoulme mentioned this pull request Feb 9, 2024
Merged
dmitryax pushed a commit that referenced this pull request Feb 16, 2024
@atoulme
[service/telemetry] delete deprecated config types (#9546)
76b017d 
**Description:**
Delete deprecated config types which have moved to
opentelemetry-go-contrib.

See #8620
for the first changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mx-psi mx-psi mx-psi approved these changes

@djaglowski djaglowski Awaiting requested review from djaglowski djaglowski is a code owner automatically assigned from open-telemetry/collector-approvers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.90.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@codeboten @mx-psi