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

Added examples for Async.WebExtensions #12644

Merged
merged 3 commits into from
Jan 27, 2022
Merged

Added examples for Async.WebExtensions #12644

merged 3 commits into from
Jan 27, 2022

Conversation

leolorenzoluis
Copy link
Contributor

@leolorenzoluis leolorenzoluis commented Jan 26, 2022

@dsyme --
Not sure if it's worth adding examples here because WebClient is deprecated in favor of HttpClient but I did it anyway.
Please see FSharp.Control.Async.WebExtensions #12124

@dnfadmin
Copy link

dnfadmin commented Jan 26, 2022

CLA assistant check
All CLA requirements met.

/// <example-tbd></example-tbd>
/// <example id="get-response">
/// <code lang="fsharp">
/// let responseStreamToString = fun (responseStream : System.Net.WebResponse) ->
Copy link
Contributor

Choose a reason for hiding this comment

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

You can add open System and open System.IO and open System.Net at the top of the sample I think. Best to keep the code crisp.

/// <example id="async-download-string">
/// <code lang="fsharp">
/// let client = new WebClient()
/// System.Uri("https://www.microsoft.com") |> client.AsyncDownloadString |> Async.RunSynchronously
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to a more neutral URL like https://www.w3.org/

/// <example id="async-download-data">
/// <code lang="fsharp">
/// let client = new WebClient()
/// client.AsyncDownloadData(Uri("https://www.microsoft.com")) |> Async.RunSynchronously |> System.Text.Encoding.ASCII.GetString
Copy link
Contributor

Choose a reason for hiding this comment

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

Likewise here, use https://www.w3.org/

/// <example id="async-download-file">
/// <code lang="fsharp">
/// let client = new System.Net.WebClient()
/// System.Uri("https://www.microsoft.com") |> fun x -> client.AsyncDownloadFile(x, "output.html") |> Async.RunSynchronouslyl
Copy link
Contributor

Choose a reason for hiding this comment

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

Likewise here, use https://www.w3.org/

Copy link
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

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

Looks great, just a few small changes requested

@leolorenzoluis
Copy link
Contributor Author

@dsyme - Thank you for reviewing. I've updated the files.

@dsyme dsyme merged commit b6e03ec into dotnet:main Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants