Skip to content

Conversation

@nagilson
Copy link
Member

It would be ideal if this could be done automatically, though per Marc it could make the migration to 9 harder so probably not.

@ghost
Copy link

ghost commented Nov 30, 2022

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@marcpopMSFT
Copy link
Member

You may want nexttargetframework to stay as 8.0. Jason added that for the transition from 7 to 8.0 as we had some tests that required 7 still and some that required 8 already. I don't think those tests will work targeting 9. The alternative is to move all tests that use next target framework currently back to current with this PR.

I like that alternative better.

@nagilson
Copy link
Member Author

@marcpopMSFT Pretty sure that's already been done, there are no references to either of the nextTFM properties.
image

@dsplaisted
Copy link
Member

dsplaisted commented Dec 1, 2022

I don't think that "Find all references" search is working. I see references still here:

[InlineData(true, true, ToolsetInfo.NextTargetFramework)]
[InlineData(true, false, ToolsetInfo.NextTargetFramework)]
[InlineData(false, false, ToolsetInfo.NextTargetFramework)]

new object[] { ToolsetInfo.NextTargetFramework }

@v-wuzhai
Copy link
Contributor

v-wuzhai commented Dec 1, 2022

The NextTargetFramework constant is also used in #29158.

@nagilson
Copy link
Member Author

nagilson commented Dec 13, 2022

~~ @dotnet/templating-engine-maintainers PTAL at the failure:
Microsoft.DotNet.Cli.New.IntegrationTests.CommonTemplatesTests.FeaturesSupport(name: \"console\", buildPass: True, framework: \"net7.0\", langVersion: null, langVersionUnsupported: False, language: null, supportsNullable: True, supportsTopLevel: True, forceDisableTopLevel: False, supportsImplicitUsings: True, supportsFileScopedNs: True) with the new ToolsetInfo.CurrentTargetFramework.

https://dev.azure.com/dnceng-public/public/_build/results?buildId=110027&view=ms.vss-test-web.build-test-results-tab
~~

JK it's been fixed.

@nagilson
Copy link
Member Author

@dotnet/dotnet-cli PTAL

@baronfel
Copy link
Member

I'm glad that we caught some build-logic updates (warning levels) with this bump :) How concerned should I be that it looks like we're losing a lot of coverage on net7.0 TFMs on tests?

@nagilson
Copy link
Member Author

nagilson commented Dec 13, 2022

I am not too concerned since they are still running in the 7x branches and theoretically any gap in 8 and 7 should be specifically tested with 7 and 8. (I'm not saying they are, but stylistically it would be the correct thing to do and it's what I've been doing for 8.0 breaking changes.)

@baronfel
Copy link
Member

@nagilson impeccable response, you are of course correct on all counts. Thank you :)

@nagilson
Copy link
Member Author

Lol! I have gotten better at covering my bases.

@nagilson nagilson merged commit 791f5ee into dotnet:main Dec 13, 2022
@marcpopMSFT
Copy link
Member

+1 to what Noah said. We should probably identify a specific set of tests that target all supported runtimes as a baseline but everything else we should roll forward each release.

@sbomer sbomer mentioned this pull request Dec 20, 2022
sbomer added a commit that referenced this pull request Dec 21, 2022
The "net7.0" TFM was dropped in the move to "net8.0" in #29316.
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.

6 participants