-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[configtls] Rename config structs for consistency #9495
[configtls] Rename config structs for consistency #9495
Conversation
I just ran this: rg "TLSClientSetting" -l --type go | xargs -I{} sh -c "gsed -i 's/TLSClientSetting/ClientConfig/g' {}"
rg "TLSServerSetting" -l --type go | xargs -I{} sh -c "gsed -i 's/TLSServerSetting/ServerConfig/g' {}"
rg "TLSSetting" -l --type go | xargs -I{} sh -c "gsed -i 's/TLSSetting/Config/g' {}" Will take care of any tests that fail (not expecting anything to fail though). |
b08a147
to
49a29fc
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9495 +/- ##
=======================================
Coverage 90.89% 90.89%
=======================================
Files 347 347
Lines 18324 18324
=======================================
Hits 16655 16655
Misses 1344 1344
Partials 325 325 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking this. We'll need to transition from the old types to the new ones, so there will have to be a little bit of duplication meanwhile.
88dfcf1
to
6df20bc
Compare
Can this be merged? or am I missing a step? |
@arjunmahishi please take a look at the conflict. |
This commit renames the following structs: * TLSClientSetting to ClientConfig * TLSServerSetting to ServerConfig * TLSSetting to Config This is based on the naming convention followed in other config packages. Fixes open-telemetry#9474
5be2d60
to
082dfaf
Compare
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just minor asks
# (Optional) One or more lines of additional information to render under the primary note. | ||
# These lines will be padded with 2 spaces and then inserted directly into the document. | ||
# Use pipe (|) for multiline entries. | ||
subtext: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please list all the renames here, e.g.:
subtext: | |
subtext: | | |
- TLSClientSetting -> ClientConfig |
Description:
Simply renames a few structs in the
configtls
package for consistence.TLSClientSetting
toClientConfig
TLSServerSetting
toServerConfig
TLSSetting
toConfig
Link to tracking Issue:
Fixes #9474