Skip to content

Adding the description for trackingIDs filter #36939

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
Closed
bhattashishMsft wants to merge 7 commits into from
Closed

Adding the description for trackingIDs filter #36939

bhattashishMsft wants to merge 7 commits into from

Conversation

@bhattashishMsft
Copy link
Contributor

@bhattashishMsft bhattashishMsft commented Aug 26, 2025

Choose a PR Template

Switch to "Preview" on this description then select one of the choices below.

Click here to open a PR for a Data Plane API.

Click here to open a PR for a Control Plane (ARM) API.

Click here to open a PR for only SDK configuration.

@github-actions
Copy link

github-actions bot commented Aug 26, 2025 •
edited
Loading

Next Steps to Merge

Next steps that must be taken to merge this PR:
  • ❌ This PR targets either the main branch of the public specs repo or the RPSaaSMaster branch of the private specs repo. These branches are not intended for iterative development. Therefore, you must acknowledge you understand that after this PR is merged, the APIs are considered shipped to Azure customers. Any further attempts at in-place modifications to the APIs will be subject to Azure's versioning and breaking change policies. Additionally, for control plane APIs, you must acknowledge that you are following all the best practices documented by ARM at aka.ms/armapibestpractices. If you do intend to release the APIs to your customers by merging this PR, add the PublishToCustomers label to your PR in acknowledgement of the above. Otherwise, retarget this PR onto a feature branch, i.e. with prefix release- (see aka.ms/azsdk/api-versions#release--branches).
  • ❌ This PR is in purview of the ARM review (label: ARMReview). This PR must get ARMSignedOff label from an ARM reviewer.
    This PR has ARMChangesRequested label. Please address or respond to feedback from the ARM API reviewer.
    When you are ready to continue the ARM API review, please remove the ARMChangesRequested label.
    Automation should then add WaitForARMFeedback label.
    ❗If you don't have permissions to remove the label, request write access per aka.ms/azsdk/access#request-access-to-rest-api-or-sdk-repositories.
    For details of the ARM review, see aka.ms/azsdk/pr-arm-review
  • ❌ The required check named Breaking Change(Cross-Version) has failed. To unblock this PR, follow the process at aka.ms/brch.


Comment generated by summarize-checks workflow run.

@github-actions github-actions bot added brownfield Brownfield services will soon be required to convert to TypeSpec. See https://aka.ms/azsdk/typespec. ARMReview resource-manager WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Aug 26, 2025
@github-actions
Copy link

github-actions bot commented Aug 26, 2025 •
edited
Loading

API Change Check

APIView identified API level changes in this PR and created the following API reviews

Language API Review for Package
Swagger Microsoft.Advisor
Go sdk/resourcemanager/advisor/armadvisor
C# Azure.ResourceManager.Advisor
Java com.azure.resourcemanager:azure-resourcemanager-advisor
JavaScript @azure/arm-advisor

@bhattashishMsft bhattashishMsft force-pushed the bhattashish/addFilterDesc branch from 4f90a48 to 847b766 Compare August 26, 2025 12:17
@josefree
Copy link
Member

josefree commented Aug 26, 2025

A PR converting this service to TypeSpec has been prepped by the Azure SDK team as part of the drive to move all existing Azure services to use TypeSpec. The PR for this is #34319. The team owning this service will need to validate the PR, address any highlighted linter failures, make any service-specific changes to the PR, have a review with ARM, and complete the PR before any additional work can be done on the service. Information regarding how to do this finalization and who to contact for assistance are at https://tspwebsitepr.z22.web.core.windows.net/typespec-azure/prs/2700/docs/migrate-swagger/01-get-started/#review-and-adjust-the-typespec. Going forward, any change to the service spec will need to be performed with TypeSpec.

@ramoka178
Copy link
Contributor

ramoka178 commented Sep 3, 2025

Choose a PR Template

Switch to "Preview" on this description then select one of the choices below.

Click here to open a PR for a Data Plane API.

Click here to open a PR for a Control Plane (ARM) API.

Click here to open a PR for only SDK configuration.

@bhattashishMsft Please fill in the form for the context of the PR to be set.
If you are creating a new api-version, let the first commit be an exact copy of the previous apiversion's files placed in the new folder for the new apiversion.
Make additional changes after that.
This way, we can compare the first commit and last commit and see what changed.

Now, if I compare to 2025-05-01-preview, it shows lot of changes in advisor. json.

@ramoka178 ramoka178 added ARMChangesRequested and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Sep 3, 2025
ramoka178
ramoka178 reviewed Sep 3, 2025
View reviewed changes
...ification/advisor/resource-manager/Microsoft.Advisor/preview/2025-08-01-preview/advisor.json
}
}
},
"/{resourceUri}/providers/Microsoft.Advisor/recommendations": {
Copy link
Contributor

@ramoka178 ramoka178 Sep 3, 2025

Choose a reason for hiding this comment

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

resourceUri

So, this can work on any azure resource scope ? want to make sure if the RP is ready for this.

ramoka178
ramoka178 reviewed Sep 3, 2025
View reviewed changes
...ification/advisor/resource-manager/Microsoft.Advisor/preview/2025-08-01-preview/advisor.json
"default": {
"description": "Error response describing why the operation failed.",
"schema": {
"$ref": "#/definitions/ArmErrorResponse"
Copy link
Contributor

@ramoka178 ramoka178 Sep 3, 2025

Choose a reason for hiding this comment

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

ArmErrorResponse

why not use from common-types.

ramoka178
ramoka178 reviewed Sep 3, 2025
View reviewed changes
...ification/advisor/resource-manager/Microsoft.Advisor/preview/2025-08-01-preview/advisor.json
"type": "object"
}
},
"tracked": {
Copy link
Contributor

@ramoka178 ramoka178 Sep 3, 2025

Choose a reason for hiding this comment

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

tracked

Enums are suggested over booleans.

Can this be modelled as a enum of saym TrackingState, with values of Enabled, Disabled etc ?

ramoka178
ramoka178 reviewed Sep 3, 2025
View reviewed changes
...ification/advisor/resource-manager/Microsoft.Advisor/preview/2025-08-01-preview/advisor.json
"name": {
"description": "The Name of the Review",
"type": "string"
}
Copy link
Contributor

@ramoka178 ramoka178 Sep 3, 2025

Choose a reason for hiding this comment

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

do not use top level Azure resource property names.

ramoka178
ramoka178 reviewed Sep 3, 2025
View reviewed changes
...ification/advisor/resource-manager/Microsoft.Advisor/preview/2025-08-01-preview/advisor.json
"properties": {
"id": {
"description": "The ARM Resource Id of the Review",
"type": "string"
Copy link
Contributor

@ramoka178 ramoka178 Sep 3, 2025

Choose a reason for hiding this comment

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

format: arm-id

ramoka178
ramoka178 reviewed Sep 3, 2025
View reviewed changes
...ification/advisor/resource-manager/Microsoft.Advisor/preview/2025-08-01-preview/advisor.json
}
}
},
"sourceSystem": {
Copy link
Contributor

@ramoka178 ramoka178 Sep 3, 2025

Choose a reason for hiding this comment

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

sourceSystem

can it be from enums ?

ramoka178
ramoka178 reviewed Sep 3, 2025
View reviewed changes
...ification/advisor/resource-manager/Microsoft.Advisor/preview/2025-08-01-preview/advisor.json
"description": "The Source System that this Recommendation originated from.",
"type": "string"
},
"notes": {
Copy link
Contributor

@ramoka178 ramoka178 Sep 3, 2025

Choose a reason for hiding this comment

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

notes

is this needed ? is localization handled ?

ramoka178
ramoka178 reviewed Sep 3, 2025
View reviewed changes
...ification/advisor/resource-manager/Microsoft.Advisor/preview/2025-08-01-preview/advisor.json
"modelAsString": true
}
},
"postponedTime": {
Copy link
Contributor

@ramoka178 ramoka178 Sep 3, 2025

Choose a reason for hiding this comment

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

postponedTime

postponedTimeInUTC ?

@microsoft-github-policy-service microsoft-github-policy-service bot added no-recent-activity There has been no recent activity on this issue. and removed no-recent-activity There has been no recent activity on this issue. labels Sep 22, 2025
@github-actions github-actions bot added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Sep 30, 2025
@github-actions github-actions bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Sep 30, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added no-recent-activity There has been no recent activity on this issue. and removed no-recent-activity There has been no recent activity on this issue. labels Oct 20, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added no-recent-activity There has been no recent activity on this issue. and removed no-recent-activity There has been no recent activity on this issue. labels Nov 3, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added no-recent-activity There has been no recent activity on this issue. and removed no-recent-activity There has been no recent activity on this issue. labels Nov 17, 2025
@microsoft-github-policy-service
Copy link
Contributor

microsoft-github-policy-service bot commented Dec 8, 2025

Hi, @@bhattashishMsft. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

@microsoft-github-policy-service microsoft-github-policy-service bot added the no-recent-activity There has been no recent activity on this issue. label Dec 8, 2025
@microsoft-github-policy-service
Copy link
Contributor

microsoft-github-policy-service bot commented Dec 22, 2025

Hi, @@bhattashishMsft. The PR will be closed since the PR has no update for 28 days. If you still need the PR review to proceed, please reopen it and @ mention PR assignee.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ramoka178 ramoka178 ramoka178 left review comments

@lizixuan1125msft lizixuan1125msft Awaiting requested review from lizixuan1125msft

@weshaggard weshaggard Awaiting requested review from weshaggard weshaggard is a code owner

@mikeharder mikeharder Awaiting requested review from mikeharder mikeharder is a code owner

Assignees

No one assigned

Labels

ARMChangesRequested ARMReview brownfield Brownfield services will soon be required to convert to TypeSpec. See https://aka.ms/azsdk/typespec. new-api-version no-recent-activity There has been no recent activity on this issue. resource-manager service-under-conversion

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@bhattashishMsft @josefree @ramoka178 @jamlai @m0salah111