-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Convert Roslyn.sln to slnx #80440
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
Convert Roslyn.sln to slnx #80440
Conversation
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.
I think @CyrusNajmabadi had a concern about this being loadable in MSBuildWorkspace, but that seems to be possible: #77326
| @@ -0,0 +1,414 @@ | |||
| <Solution> | |||
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.
Just curious, this slnx file was produced by dotnet sln migrate or some other mechanism? #Closed
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.
Yes, no manual changes appear to be needed. I will do a few more spot checks before merging though (verify count of projects before and after, verify numbers of tests running in CI.)
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.
Number of tests and number of projects both check out.
| 1. [PowerShell 5.0 or newer](https://docs.microsoft.com/en-us/powershell/scripting/setup/installing-windows-powershell). If you are on Windows 10, you are fine; you'll only need to upgrade if you're on earlier versions of Windows. The download link is under the ["Upgrading existing Windows PowerShell"](https://docs.microsoft.com/en-us/powershell/scripting/install/installing-windows-powershell?view=powershell-6#upgrading-existing-windows-powershell) heading. | ||
| 1. Run Restore.cmd | ||
| 1. Open Roslyn.sln | ||
| 1. Open Roslyn.slnx |
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: Not related to PR, we may want to mention Compilers.slnf for compiler work
jcouv
left a comment
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.
LGTM Thanks (commit 3)
jcouv
left a comment
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.
LGTM Thanks (commit 5)
| @@ -1,2390 +0,0 @@ | |||
| Microsoft Visual Studio Solution File, Format Version 12.00 | |||
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.
Looks like there is still a mention of Roslyn.sln in .github\copilot-instructions.md
| <BuildDependency Project="src/EditorFeatures/VisualBasic/Microsoft.CodeAnalysis.VisualBasic.EditorFeatures.vbproj" /> | ||
| </Project> | ||
| <Project Path="src/Tools/BuildActionTelemetryTable/BuildActionTelemetryTable.csproj" /> | ||
| <Project Path="src/Tools/BuildBoss/BuildBoss.csproj" /> |
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.
We seem to have other .sln files in the repo (like eng\eng.sln, src\Tools\BuildBoss\BuildBoss.sln, ...) but that's presumably out of scope of this PR.
|
Note: the CI failure is unrelated, should be fixed by #81048 |
|
I'll plan to merge in the morning and send out notice to x-team. |
Moving to slnx improves the clarity and maintainability of the solution file.
See also dotnet/sdk#49291 for another example of such a conversion.
Since this change may be disruptive (will break peoples Recent Solutions list etc when pulling latest main) we will want some team discussion and heads-up before taking the change.