Conversation
|
@nguerrera @dsplaisted @srivatsn can one of you review & approve? |
dsplaisted
left a comment
There was a problem hiding this comment.
LGTM.
I see update to the dotnet add tests. Were there no tests covering this Guid for the migrate scenario?
Also, just to be sure, it would be good to test opening the result of dotnet migrate and dotnet sln add (or whatever the syntax is) in VS and make sure that the issues described in #5665 are fixed.
|
@dsplaisted this change only covers the |
|
No, migration actually shares this code. This will fix the migration issue as well. But the migration tests don't check that GUID explicitly. We rely on the sln add tests for that. @jgoshi can correct me if I am wrong. |
|
@livarcocc is correct. Whenever migration needs to add a project to the sln file it calls dotnet sln add. |
|
I also manually tried the scenario specified by And this fixes it. |
Fixes #5688
@piotrpMSFT @livarcocc @krwq @jonsequitur @eerhardt
The work to have the DefaultProjectTypeGuid isn't available yet. Here is the PR:
dotnet/msbuild#1607
And work also has to be done for F#. What we will eventually do in the CLI is to throw if there isn't an explicit ProjectTypeGuid or a DefaultProjectTypeGuid defined. But until the work for writing DefaultProjectTypeGuid goes in we need to burn in some default. This work changes the default to be the C# one. We will still be wrong in other cases (like for a VB project or F# project or any other type of project). But at least default to C# rather than CPS.