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

split config/configauth module #7910

Merged

Conversation

codeboten
Copy link
Contributor

Linked issue #7895

Follow up to #7908

@codeboten codeboten force-pushed the codeboten/split-config-mod-configauth branch from a902c43 to ca0c241 Compare June 15, 2023 22:53
Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

Apparently, there are some formatting issues. Nothing that would block this PR from being merged, but nice to have them fixed anyway. If you prefer to fix them at a later time, let me know and I'll approve the PR.

component/go.mod Outdated
@@ -35,6 +35,8 @@ require (
gopkg.in/yaml.v3 v3.0.1 // indirect
)



Copy link
Member

Choose a reason for hiding this comment

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

Was this added by a tool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was added in the previous PR by crosslink removing other dependencies and leaving the new lines until i re-ran the tool

go 1.19



Copy link
Member

Choose a reason for hiding this comment

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

This go.mod looks strange now: perhaps add the direct dependencies first, the transitive after that, and the overrides at the bottom?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the replace, ptal

extension/auth/go.mod Outdated Show resolved Hide resolved
extension/go.mod Outdated Show resolved Hide resolved
Linked issue: open-telemetry#7895

Signed-off-by: Alex Boten <aboten@lightstep.com>
@codeboten codeboten force-pushed the codeboten/split-config-mod-configauth branch from ca0c241 to 6aafddb Compare June 16, 2023 19:32
@codeboten codeboten marked this pull request as ready for review June 16, 2023 19:32
@codeboten codeboten requested review from a team and jpkrohling June 16, 2023 19:32
Copy link
Contributor Author

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Addressed all the newlines, ptal @jpkrohling

Signed-off-by: Alex Boten <aboten@lightstep.com>
@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.05 🎉

Comparison is base (f482916) 91.06% compared to head (c755bd3) 91.12%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7910      +/-   ##
==========================================
+ Coverage   91.06%   91.12%   +0.05%     
==========================================
  Files         298      298              
  Lines       14912    14912              
==========================================
+ Hits        13580    13588       +8     
+ Misses       1053     1047       -6     
+ Partials      279      277       -2     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@codeboten codeboten merged commit 5f8213f into open-telemetry:main Jun 16, 2023
@codeboten codeboten deleted the codeboten/split-config-mod-configauth branch June 16, 2023 21:51
@github-actions github-actions bot added this to the next release milestone Jun 16, 2023
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