Skip to content

Conversation

@svick
Copy link
Contributor

@svick svick commented Dec 2, 2018

Fixes dotnet/dotnet-api-docs#1045.

Also switches from explicitly calling Dispose to using, because I think that's a better practice.

In the process of testing this change, I used the following csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>net45;netcoreapp1.0</TargetFrameworks>
    <LangVersion>7.1</LangVersion>
  </PropertyGroup>

  <ItemGroup Condition="'$(TargetFramework)' == 'net45'">
    <Reference Include="System.Net.Http" />
  </ItemGroup>

</Project>

Would it be useful if I included it in this PR?

@richlander
Copy link
Member

Separately, should we add a usage of HttpClientFactory?

@svick
Copy link
Contributor Author

svick commented Dec 3, 2018

@richlander

  1. Isn't that specific to ASP.NET Core? I mean, it's technically usable outside of it, but is using it actually a recommended practice in, say, WPF or Xamarin?
  2. Wouldn't that require lots of dependency injection-related code? I think that doesn't belong here.

Though adding a link to a document about it might make sense.

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes @svick

I've approved this change, and I'll :shipit: now.

Thanks again for all your contributions.

@BillWagner BillWagner merged commit 3bc5a5e into dotnet:master Dec 3, 2018
@BillWagner
Copy link
Member

@svick:

Would it be useful if I included it (the csproj) in this PR?

What we've been doing as time permits is migrating the snippets into the dotnet/samples repository, and making them part of a buildable project. That does require 2 PRs: one in samples to create the new sample, and a separate one in dotnet/docs to refer to the new sample and remove the old code. I didn't want to hold up a good PR on that process though.

@mairaw mairaw added the bug Something isn't working label Dec 3, 2018
@svick svick deleted the httpclient-dispose branch August 17, 2019 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants