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

Correct Sample README Product slug #20679

Merged
merged 5 commits into from
Apr 15, 2021

Conversation

YijunXieMS
Copy link
Contributor

@YijunXieMS YijunXieMS commented Apr 14, 2021

"products" of sample README must be from the slugs of Taxonomy Service. Otherwise, the sample doc generation will have an error and won't proceed to process other SDKs' sample docs.

closes #20685

@YijunXieMS YijunXieMS added the Client This issue points to a problem in the data-plane of the library. label Apr 14, 2021
@YijunXieMS YijunXieMS self-assigned this Apr 14, 2021
@ghost ghost added the Schema Registry label Apr 14, 2021
@@ -4,7 +4,7 @@ languages:
- java
products:
- azure
- azure-data-schemaregistry-avro
- azure-event-hubs
Copy link
Member

Choose a reason for hiding this comment

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

Why is schema registry using event hubs tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

schema registry isn't a listed product in Taxonomy Service so the doc generation has an error.

schema registry is a sub-service of event hubs so I use this closest parent. If I'm wrong, we can use the root "azure".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per offline discussion, schema registry isn't a sub service of eventhubs. Use "azure" for now until schema-registry itself is listed.

Copy link
Member

Choose a reason for hiding this comment

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

It may not be, but that doesn't mean it's not related. These are slugs for where customers should see the sample, so if it's related, use the slug. These aren't categorizations of the sample, but a list of related products.

@@ -4,7 +4,7 @@ languages:
- java
products:
- azure
- azure-data-schemaregistry
- azure-event-hubs
Copy link
Member

Choose a reason for hiding this comment

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

Same here - should this be reverted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above. Use "azure" only.

@YijunXieMS YijunXieMS requested a review from lilyjma April 14, 2021 22:52
Copy link
Member

@gapra-msft gapra-msft left a comment

Choose a reason for hiding this comment

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

storage side looks good to me

@@ -4,7 +4,6 @@ languages:
- java
products:
- azure
- azure-data-schemaregistry-avro
Copy link
Member

Choose a reason for hiding this comment

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

Why not azure-event-hubs since it's related? It's not like this ties the sample to that, but shows up in more places. It's just a list of what's related.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ping @srnagar @lilyjma. what do you think of it?

Copy link
Member

Choose a reason for hiding this comment

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

The other languages have also already used this slug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll change to event-hubs to align with other languages. We can change it later if we have a new guideline.

@gapra-msft
Copy link
Member

@YijunXieMS When will you be merging this PR? It is currently blocking me from merging one of my PRs that I need to merge before I do a release tomorrow

@YijunXieMS YijunXieMS merged commit 3f4315f into Azure:master Apr 15, 2021
@YijunXieMS YijunXieMS deleted the fix_taxonomy_slugs branch April 15, 2021 23:58
@YijunXieMS
Copy link
Contributor Author

@YijunXieMS When will you be merging this PR? It is currently blocking me from merging one of my PRs that I need to merge before I do a release tomorrow

Merged.

@gapra-msft
Copy link
Member

@YijunXieMS When will you be merging this PR? It is currently blocking me from merging one of my PRs that I need to merge before I do a release tomorrow

Merged.

Thank you!

benbp pushed a commit that referenced this pull request Apr 28, 2021
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. Schema Registry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some samples have invalid product slugs
5 participants