-
Notifications
You must be signed in to change notification settings - Fork 1.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
Add support for dotnet .slnx
solution files
#11694
base: main
Are you sure you want to change the base?
Conversation
1606ce5
to
61e9d8c
Compare
61e9d8c
to
d83d929
Compare
@austindrenski One required spec is failing, you may like to check this. |
(solutionPath, """ | ||
<Solution> | ||
<Folder Name="/src/"> | ||
<Project Path="src/project.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.
Could you use a backslash here? The updater ultimately runs on Linux and unit tests run on Linux, too, and using "src\project.csproj"
instead of "src/project.csproj"
would help make sure the paths were normalized.
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="Some.Package" /> |
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.
Add the Version
attribute here and drop the files Directory.Build.props
, Directory.Packages.props
, global.json
, and dotnet-tools.json
. I'd rather the test be constrained to just validating .slnx
expansion.
("src/project.csproj", """ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<TargetFrameworks>net7.0;net8.0</TargetFrameworks> |
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 a single target framework should suffice since the desired behavior is verifying .slnx
expansion and nothing else.
@@ -232,6 +236,30 @@ private static ImmutableArray<string> ExpandEntryPointsIntoProjects(IEnumerable< | |||
filesToExpand.Push(project.AbsolutePath); | |||
} | |||
} | |||
else if (extension == ".slnx") | |||
{ | |||
var projects = XElement |
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.
Use the official .slnx
parser added in this PR instead of direct XML loading; I'm not paid enough to edit this when the official parser does something unexpected in 3 years :)
Fixes: #11671
What are you trying to accomplish?
.slnx
solution formatAnything you want to highlight for special attention from reviewers?
This format is relatively new, but at this point Visual Studio, JetBrains Rider, and the dotnet CLI all support building both
.sln
and.slnx
files.I've got a group of very eager devs, myself included, who are migrating ancient
.sln
files to.slnx
format, and so far the biggest blocker is a lack of dependabot support.So, recognizing that this PR may not cover all of the interesting corner cases, I'm hoping it might at least get the ball rolling since, as far as I can tell, dependabot only uses
.sln
files as graph roots to crawl for project files, and that's (breathtakingly) straightforward in the new XML-based.slnx
format.How will you know you've accomplished your goal?
.slnx
is already in use.Checklist