-
Notifications
You must be signed in to change notification settings - Fork 165
API review of Steeltoe.Common #1334
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
Conversation
…sages: - File/directory name can consist of whitespace - Configuration key can consist of whitespace - Logger category name can consist of whitespace - Assembly/type/member name can NOT consist of whitespace - Hostname/url can NOT consist of whitespace - Eureka app-name/instance-ID/VIP-address can NOT consist of whitespace Use ToArray() instead of ToList(), which is slightly more efficient
The following configuration keys are no longer being read, as they were never meant to be: - application:name - application:uris - consul:serviceName - management:name - spring:application:instance_id Renames: - IServiceCollection.RegisterDefaultApplicationInstanceInfo -> AddApplicationInstanceInfo - IServiceCollection.RegisterCloudFoundryApplicationInstanceInfo -> AddCloudFoundryOptions (unified with other extension methods)
…c-comments, make Add*ActuatorServices extension methods internal
…nsafe when used from multiple Steeltoe components)
…ors, unify test contributors into single type
/azp run cleanup-code |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run Steeltoe.All |
Azure Pipelines successfully started running 1 pipeline(s). |
Code cleanup successfully reformatted files, but there were no changes to push. |
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.
Looks really good to me, a few questions, comments and minor changes to make
src/Configuration/test/CloudFoundry.Test/CloudFoundryServiceCollectionExtensionsTest.cs
Outdated
Show resolved
Hide resolved
src/Management/src/Endpoint/ThreadDump/EndpointServiceCollectionExtensions.cs
Show resolved
Hide resolved
src/Management/src/Endpoint/SpringBootAdminClient/ConfigureSpringBootAdminClientOptions.cs
Show resolved
Hide resolved
src/Common/src/Common/Configuration/PropertyPlaceHolderHelper.cs
Outdated
Show resolved
Hide resolved
src/Configuration/src/Placeholder/PlaceholderResolverProvider.cs
Outdated
Show resolved
Hide resolved
src/Configuration/src/ConfigServer/ConfigServerConfigurationProvider.cs
Outdated
Show resolved
Hide resolved
Azure Pipelines successfully started running 1 pipeline(s). |
|
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
Description
Public API review of Steeltoe.Common (first part). See the individual commit messages for details.
Closes #898, closes #897, closes #825, closes #928, closes #916, closes #929, closes #941, closes #946, closes #947, closes #590, closes #962, closes #1257, closes #1271.
Quality checklist
If your change affects other repositories, such as Documentation, Samples and/or MainSite, add linked PRs here.