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

[MetricsAdvisor] Renamed DataSourceCredential types #22119

Merged
merged 3 commits into from
Jun 24, 2021

Conversation

kinelski
Copy link
Member

Feedback from architects + cross-language consistency.

  • The abstract type DataSourceCredential has been renamed to DataSourceCredentialEntity.
  • All of its concrete subtypes followed the pattern <credential-type>DataSourceCredential. They have been renamed to DataSource<credential-type>. Example: "ServicePrincipalDataSourceCredential" => "DataSourceServicePrincipal".

@kinelski kinelski added the Client This issue points to a problem in the data-plane of the library. label Jun 23, 2021
@kinelski kinelski requested a review from maririos June 23, 2021 22:36
@kinelski kinelski self-assigned this Jun 23, 2021
- The whole `DatasourceCredential` API has been renamed to `DataSourceCredential`. This includes renames in types, methods, and properties.
- The whole `DatasourceCredential` API has been renamed to `DataSourceCredential`. This includes renames in methods, method parameters, and properties.
- Renamed class `DatasourceCredential` to `DataSourceCredentialEntity`.
- Renamed class `DataLakeGen2SharedKeyDatasourceCredential` to `DataSourceDataLakeGen2SharedKey`.
Copy link
Member

Choose a reason for hiding this comment

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

ohh cool. Just for my own learning, what was the reason for moving DataSource to the beginning of the types?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't like ending in "Credential" that much because this can get mixed up with credential types in Identity. I'm okay with just appending "CredentialIdentity" to everything, but other languages are taking this approach, so I'm following along for consistency. The reasoning is something like "DataSourceSqlConnectionString" is a "data source's SQL connection string".

In terms of advantage, I can only think of:

  • Types names get shorter (I've heard that DataLakeGen2SharedKeyDataSourceCredential is too long more than once).
  • All credential types get organized in the same place because of alphabetical order.

@kinelski kinelski merged commit 69917f9 into Azure:main Jun 24, 2021
@kinelski kinelski deleted the ma-renames2 branch June 24, 2021 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Cognitive - Metrics Advisor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants