-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Use $(TargetFramework) in the SDK when only targeting a single TFM #258
Conversation
Specify $(TargetFramework) instead of $(TargetFrameworks). All dependencies are declared in PackageReference. Tests use $(PackageTargetFallback) in place of project.json imports.
/cc @dotnet/project-system @piotrpMSFT |
@mlorbetske @abpiskunov @phenning we should update all the web templates to use TargetFramework instead of TargetFrameworks as well if they have just one TF. |
<FluentAssertionsVersion>4.0.0</FluentAssertionsVersion> | ||
<FluentAssertionsJsonVersion>4.12.0</FluentAssertionsJsonVersion> | ||
<NewtonsoftJsonVersion>9.0.1</NewtonsoftJsonVersion> | ||
<DotNetCliUtilsVersion>1.0.0-preview2-003121</DotNetCliUtilsVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: whitespace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops.
Is something setting Configuration env var in the SDK build too? I see we have SkipInvalidConfigurations in restore command and that's still just downgrading ~20 errors to warnings. I think that:
|
I think it's fine to update it to set single target framework after props since that was because of the nuget bug. We still need to verify if multiple targetframeworks works with web, but this doesn't impact the template any longer since templates are supposed to be single target framework now and with the nuget bug fixed, this can be done in the right place. |
We pass the Configuration into our call into MSBuild: https://github.com/dotnet/sdk/blob/master/build.ps1#L62 We need to fix the bug: #203 |
Agreed. Until then and as long as we're having to work around it with /p:SkipInvalidConfigurations=true, can we also pass /p:_InvalidConfigurationWarning=false. Or can we not pass configuration to the restore invocations? I find it really distracting for every build to have a sea of yellow and more critically, it lets new warnings slip in unnoticed. |
Adding support for PB_SignType and PB_SkipTests
This + retry seems to get uploads pretty reliable for ghcr.io and azurecr.io, at least in my testing.
There were 2 places I didn't change that should be.
@nguerrera @srivatsn