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.

Sign up for GitHub

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

Jump to bottom

Re-enable verify-sample template #18066

Closed
heaths opened this issue Apr 15, 2021 · 7 comments
Closed

Re-enable verify-sample template #18066

heaths opened this issue Apr 15, 2021 · 7 comments
Assignees
@heaths @scbedd
Labels
Central-EngSys This issue is owned by the Engineering System team. EngSys This issue is impacting the engineering system.

Comments

@heaths
Copy link
Member

heaths commented Apr 15, 2021

Due to #17991 and related issues, I had to disable the new verify-sample template because it's blocking the sync PR to fix other issues.
@heaths heaths added the EngSys This issue is impacting the engineering system. label Apr 15, 2021
@heaths heaths self-assigned this Apr 15, 2021
heaths added a commit that referenced this issue Apr 15, 2021
@heaths
Disable verify-samples template
f53a0cd 
Opened #18066 to track re-enabling it.
@heaths heaths mentioned this issue Apr 15, 2021
Closed
azure-sdk added a commit that referenced this issue Apr 15, 2021
@azure-sdk @heaths
Sync eng/common directory with azure-sdk-tools for PR 1559 (#18064)
f394ae8 
* Handle empty files in Test-SampleMetadata

Relates to Azure/azure-sdk-for-java#20684

* Remove condition from verify-samples

* Use forward slash in path

* Fix directory typo

* Remove invalid 'telemetry' product slug

Relates to #17991

* Disable verify-samples template

Opened #18066 to track re-enabling it.

Co-authored-by: Heath Stewart <heaths@microsoft.com>
@heaths
Copy link
Member Author

heaths commented Apr 15, 2021

@weshaggard @scbedd do we want to fix all errors first, or enable this now to stem the tide and effectively force the next PRs for the following files to fix the issues for their service directory before merging?

dir sdk -filter *.md -recurse | eng\common\scripts\Test-SampleMetadata.ps1 -AllowParentProducts -passthru -ea Ignore | ft InvalidProducts, @{l='Path'; e={$_ | resolve-path -relative}}
InvalidProducts                        Path
---------------                        ----
{azure-identity}                       .\sdk\identity\azure-identity\samples\README.md
{azure-monitor-opentelemetry-exporter} .\sdk\monitor\azure-monitor-opentelemetry-exporter\samples\traces\README.md
{azure-schema-registry}                .\sdk\schemaregistry\azure-schemaregistry\samples\README.md
{azure-schema-registry-avro-serialize… .\sdk\schemaregistry\azure-schemaregistry-avroserializer\samples\README.md

@weshaggard
Copy link
Member

If they are easy enough to fix I'd like to fix them before merging.

@heaths
Copy link
Member Author

heaths commented Apr 15, 2021

There's currently no replacements for telemetry and identity. The other two we had decided on over email, though Java wasn't crazy about putting the latter two in Event Grid.

@heaths
Copy link
Member Author

heaths commented Apr 15, 2021

Based on an internal email thread, I recommend:

azure-identity -> azure-sdks
azure-monitor-opentelemetry-exporter -> azure-monitor
azure-schema-registry* -> azure-event-hubs

@kristapratico
Copy link
Member

@rakshith91 says that azure-monitor-opentelemetry-exporter should not go under azure-monitor. He's checking right now, but thinks it should go under azure-core which makes me think that azure-sdks might make the most sense.

@kristapratico
Copy link
Member

^ sorry I confused the opentelemetry libraries when asking Rakshith. azure-monitor is appropriate for azure-monitor-opentelemetry-exporter. I opened a PR here to make the changes - #18081 (azure-identity is in another PR but should be ready to merge).

@scbedd scbedd self-assigned this Mar 4, 2022
@scbedd scbedd added the Central-EngSys This issue is owned by the Engineering System team. label Mar 4, 2022
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-python that referenced this issue Mar 28, 2022
CodeGen from PR 18066 in Azure/azure-rest-api-specs
e2da1bd 
Adding NSP child resources, profile and access rule, to swagger (Azure#18066)

* Defining the APIs for the new NSP resource

* Adding NSP child resources - profile and access rule

* Fixing the model errors, prettier-fix and typos

* Fixing the typo in the spec file for NSP

* Adding definitions for resource association

* Prettier fix on the spec and example files

* Fix typos, and removing the apiVersion on the top level

* Handling the review comments

* Correcting the subId in the path

* Adding the NSP preview version for go in the read me

Co-authored-by: Hari Prasad Perabattula <haperaba@microsoft.com>
Co-authored-by: Hari Prasad Perabattula <Hari.Perabattula@Microsoft.com>
@scbedd
Copy link
Member

scbedd commented Mar 15, 2023

Closing as Krista's PRs have merged.

@scbedd scbedd closed this as completed Mar 15, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@heaths heaths

@scbedd scbedd

Labels
Central-EngSys This issue is owned by the Engineering System team. EngSys This issue is impacting the engineering system.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@heaths @weshaggard @kristapratico @scbedd