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

Fix condition in package reference list and update dotnet cli version from 1.0.4 to 2.0.0 for non-Windows system #548

Merged
merged 1 commit into from Sep 15, 2017

Conversation

Projects
None yet
4 participants
@Ky7m
Collaborator

Ky7m commented Sep 14, 2017

No description provided.

@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Sep 14, 2017

@Ky7m,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

dnfclas commented Sep 14, 2017

@Ky7m,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@AndreyAkinshin

This comment has been minimized.

Show comment
Hide comment
@AndreyAkinshin
Member

AndreyAkinshin commented Sep 15, 2017

@Ky7m, thanks!

@AndreyAkinshin AndreyAkinshin merged commit 405c4c4 into dotnet:master Sep 15, 2017

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@@ -23,7 +23,7 @@
<Reference Include="System" />
<Reference Include="Microsoft.CSharp" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp1.1' Or '$(TargetFramework)' == 'netcoreapp1.1' ">
<ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp1.1' Or '$(TargetFramework)' == 'netcoreapp2.0' ">

This comment has been minimized.

@adamsitnik

adamsitnik Sep 15, 2017

Member

good catch! I have no idea how did it work without it ;)

@adamsitnik

adamsitnik Sep 15, 2017

Member

good catch! I have no idea how did it work without it ;)

This comment has been minimized.

@Ky7m

Ky7m Sep 15, 2017

Collaborator

@adamsitnik, does TargetFrameworkIdentifier in condition can simplify clause, example
https://github.com/aspnet/MusicStore/blob/dev/samples/MusicStore/MusicStore.csproj#L20,L24
what do you think?

@Ky7m

Ky7m Sep 15, 2017

Collaborator

@adamsitnik, does TargetFrameworkIdentifier in condition can simplify clause, example
https://github.com/aspnet/MusicStore/blob/dev/samples/MusicStore/MusicStore.csproj#L20,L24
what do you think?

This comment has been minimized.

@AndreyAkinshin

AndreyAkinshin Sep 16, 2017

Member

It makes much more sense to me. I think it's a good idea to use TargetFrameworkIdentifier everywhere (not only for .NETCoreApp, but also for .NETFramework).

@AndreyAkinshin

AndreyAkinshin Sep 16, 2017

Member

It makes much more sense to me. I think it's a good idea to use TargetFrameworkIdentifier everywhere (not only for .NETCoreApp, but also for .NETFramework).

This comment has been minimized.

@adamsitnik

adamsitnik Sep 16, 2017

Member

@Ky7m very good idea!

@adamsitnik

adamsitnik Sep 16, 2017

Member

@Ky7m very good idea!

This comment has been minimized.

@Ky7m

Ky7m Sep 16, 2017

Collaborator

Great, let me grab this item and I'll send a PR

@Ky7m

Ky7m Sep 16, 2017

Collaborator

Great, let me grab this item and I'll send a PR

@Ky7m Ky7m deleted the Ky7m:target-framework-fix branch Sep 15, 2017

@AndreyAkinshin AndreyAkinshin added this to the v0.10.10 milestone Oct 29, 2017

alinasmirnova added a commit to alinasmirnova/BenchmarkDotNet that referenced this pull request Sep 22, 2018

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