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

Adjust APIs in preparation to UX study #22260

Merged

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Jun 28, 2021

Add more to string implementations, snippets and turn advanced properties into methods so it's easier to find the main query result.

@ghost ghost added the Azure.Core label Jun 28, 2021
@pakrym pakrym requested a review from christothes June 28, 2021 22:19
@pakrym pakrym requested a review from tg-msft as a code owner June 28, 2021 22:32
@@ -81,7 +81,7 @@ protected LogsQueryClient()
/// <summary>
/// Executes the logs query.
/// </summary>
/// <param name="workspace">The workspace to include in the query.</param>
/// <param name="workspace">The workspace id to include in the query (<c>xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx</c>).</param>
Copy link
Member

Choose a reason for hiding this comment

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

should we name this workspaceId ?

Copy link
Member

Choose a reason for hiding this comment

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

Will this always be a GUID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The service defines the parameter as a string so no guarantees although I doubt it would change.

Copy link
Member

Choose a reason for hiding this comment

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

It's system defined though, correct? The only reason I was asking is that we have examples of services that take a any string value, as long as that string is a valid GUID 😄 . In that case we just typed it as GUID to avoid pain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'll chat with the team about it.

@@ -159,7 +159,31 @@ public virtual async Task<Response<LogsQueryResult>> QueryAsync(string workspace
}

/// <summary>
/// Submits the batch query.
/// Submits the batch query. Use the <see cref="LogsBatchQuery"/> to compose a batch query.
/// <code snippet="Snippet:BatchQuery" language="csharp">
Copy link
Member

Choose a reason for hiding this comment

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

Should all of these code snippets also be wrapped in <example>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want them to show in VS so embedded the <code> directly into the summary.

Copy link
Member

Choose a reason for hiding this comment

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

Do both?

Copy link
Contributor Author

@pakrym pakrym Jun 29, 2021

Choose a reason for hiding this comment

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

Then it would be duplicated on the doc page. Never tried it though. What benefits do you think it would have?

sdk/monitor/Azure.Monitor.Query/src/MetricsQueryClient.cs Outdated Show resolved Hide resolved
pakrym and others added 2 commits June 29, 2021 14:09
Co-authored-by: Scott Addie <10702007+scottaddie@users.noreply.github.com>
@pakrym pakrym enabled auto-merge (squash) June 29, 2021 22:01
@pakrym pakrym merged commit f456bc7 into Azure:main Jun 29, 2021
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-net that referenced this pull request Mar 7, 2023
Rp soft delete (Azure#22260)

* Adds base for updating Microsoft.RecoveryServices from version stable/2023-01-01 to version 2023-02-01

* Updates readme

* Updates API version in new specs and examples

* Adds RP Soft delete related API changes

* added ExtendedLocation to IaasVMRestoreRequest (Azure#22468)

* fixed lint error (Azure#22483)

* - adding changes for private endpoint support in get, list Rp and trigger restore.

* -fixing data type

* fixing validation errors

* Added changed for Secured VM details and Security Type (Azure#22848)

---------

Co-authored-by: shesamian <107954668+shesamian@users.noreply.github.com>
Co-authored-by: Arpit Jain <46751982+arpja@users.noreply.github.com>
Co-authored-by: Saksham Arora <125243942+sakshamarora9575@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants