Conversation
|
|
||
| <!-- | ||
| This props file manages the target framework properties for Roslyn. The strategy for these properties | ||
| is covered in "docs/contributing/target-framework-strategy.md". Please see that for documentation |
There was a problem hiding this comment.
Did you check this file to see if needed updates based on the TFM change?
There was a problem hiding this comment.
I think I don't understand that doc. What is "current servicing target framework" and why does .NET SDK require that? For source build, it says $(NetCurrent) and $(NetPrevious) are required, but I'm not sure how to determine those - looking at vmr binlogs, it seems those might be net10 and net9 - so does that mean we need to keep net9? Or perhaps it would be enough to add net9.0 to <NetRoslynSourceBuild>?
There was a problem hiding this comment.
vmr/source build seems to be fine with the net9 removal: dotnet/dotnet#4152
There was a problem hiding this comment.
That doc is an attempt to explain
- What TFMs our product needs to build for.
- What products are forcing us to build that set of TFMs.
- Attempt to explain a bit how the actual build logic connects to these items
As part of this change I would expect this line to be updated
.NET SDK: requires us to ship compilers on current servicing target framework (presently net9.0)
Essentially the .NET SDK drives one of our TFM requirements. Now the requirement is net10.0 (and soon net11.0). Hence it's okay for you to delete net9.0 from our code base. But should update this doc to keep it fresh.
When I didn't have this doc, I got lots of requests to add it. Hence want to keep it up to date
There was a problem hiding this comment.
Yes, the doc is great, I was just a bit confused how to determine the requirements (and hence how to update them when changing the TFMs), but I think I understand it now, will update it, thanks.
There was a problem hiding this comment.
Looking at the doc more, there's instructions for updating TFMs: https://github.com/dotnet/roslyn/blob/main/docs/contributing/target-framework-strategy.md#checklist-for-updating-tfms-once-a-year
Did we follow all the steps for this PR? In particular, I there's still NET9_0_OR_GREATER left in the code that needs to be updated.
There was a problem hiding this comment.
Did we follow all the steps for this PR?
It looks like most of those steps should have been followed when adding a new TFM (which I'm not doing in this PR, I'm merely removing an old one), but I will take a look, thanks.
There was a problem hiding this comment.
MSBuildWorkspace: requires to ship a process that must be usable on the lowest supported SDK (presently
net6.0)
I'm not sure if this is correct; @jasonmalinowski?
There was a problem hiding this comment.
Ah yes, we bumped it to net8.0.
|
/pr-val |
|
View PR Validation Run triggered by @jjonescz Parameters
|
7 participants
Follow up on #81545 (comment).
For reference, checklist for updating TFM once a year: https://github.com/dotnet/roslyn/blob/main/docs/contributing/target-framework-strategy.md#checklist-for-updating-tfms-once-a-year