-
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
Added new default functions for configgrpc
file
#9819
Added new default functions for configgrpc
file
#9819
Conversation
config/configgrpc/configgrpc.go
Outdated
@@ -90,12 +95,27 @@ type ClientConfig struct { | |||
Auth *configauth.Authentication `mapstructure:"auth"` | |||
} | |||
|
|||
// NewDefaultClientConfig() creates a new ClientConfig with any default values set | |||
func NewDefaultClientConfig() *ClientConfig { | |||
return &ClientConfig{ |
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.
This needs to use `configtls.NewDefaultClientConfig once it exists.
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.
Concerning this too, can i get a link to the issue that will resolve this so I can work on it
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.
I have prepared a branch that will set default functions for configtls
, but an issue has to first be created. Or should i go ahead and create the issue, then link a PR for the issue? @TylerHelmuth
config/configgrpc/configgrpc.go
Outdated
@@ -90,12 +95,27 @@ type ClientConfig struct { | |||
Auth *configauth.Authentication `mapstructure:"auth"` | |||
} | |||
|
|||
// NewDefaultClientConfig() creates a new ClientConfig with any default values set | |||
func NewDefaultClientConfig() *ClientConfig { | |||
return &ClientConfig{ |
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.
This function will need to call configauth.NewDefaultAuthentication once it exists.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9819 +/- ##
=======================================
Coverage 91.12% 91.13%
=======================================
Files 353 353
Lines 18723 18742 +19
=======================================
+ Hits 17062 17081 +19
Misses 1333 1333
Partials 328 328 ☔ View full report in Codecov by Sentry. |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Hello @TylerHelmuth, is there anything i need to do? |
@AkhigbeEromo We are waiting for #9658 to be merged |
@AkhigbeEromo this is ready to use the |
@TylerHelmuth Thank you very much, will add it now |
I will have to rename this PR so I do not encounter the same issues @TylerHelmuth |
d7f6a75
to
e8cabb7
Compare
Description:
Added newDefault methods for structs in configgrpc package
Link to tracking Issue:
Closes #9654
Testing: Tests were added for the NewDefault functions