dotnet / cli-migrate

MIT License
9 stars 15 forks source link

Migration doesn't handle resources file. #38

Open livarcocc opened 7 years ago

livarcocc commented 7 years ago

From @harshjain2 on November 22, 2016 2:43

dotnet migrate doesn't create entry for resource (resx) file in csproj.

Steps to reproduce

  1. Create an xproj based project and add resource (resx) file.
  2. Execute dotnet migrate project.json of this project

Expected behavior

Generated csproj file has entry for resx file

<ItemGroup>
    <Compile Update="Resources\Resources.Designer.cs">
      <DesignTime>True</DesignTime>
      <AutoGen>True</AutoGen>
      <DependentUpon>Resources.resx</DependentUpon>
    </Compile>
  </ItemGroup>
  <ItemGroup>
    <EmbeddedResource Update="Resources\Resources.resx">
      <Generator>ResXFileCodeGenerator</Generator>
      <LastGenOutput>Resources.Designer.cs</LastGenOutput>
    </EmbeddedResource>
  </ItemGroup>

Actual behavior

Generated csproj file doesn't have entry for resx file.

Environment data

dotnet --info output:

.NET Command Line Tools (1.0.0-preview3-004056)

Product Information: Version: 1.0.0-preview3-004056 Commit SHA-1 hash: ccc4968bc3

Runtime Environment: OS Name: Windows OS Version: 10.0.14393 OS Platform: Windows RID: win10-x64

Copied from original issue: dotnet/cli#4803

livarcocc commented 7 years ago

From @piotrpMSFT on November 22, 2016 8:58

@nguerrera currently migrate adds:

<ItemGroup>
    <Compile Include="**\*.cs" />
    <EmbeddedResource Include="**\*.resx" />
    <EmbeddedResource Include="compiler\resources\**\*" />
</ItemGroup>

This is not equivalent to what is suggested by @harshjain2. What does the SDK expect?

livarcocc commented 7 years ago

From @nguerrera on November 22, 2016 17:25

The SDK doesn't have fixed expectations here, but for the VS generation of strongly typed resources to work the metadata @harshjain2 shows is needed. dotnet/sdk#94 tracks moving the generation to the build and in the interim adjusting the templates so that an add resx gesture in VS gives the right metadata. We may want some interim changes to migration too. Once everything is in the build, I would expect things to work by default with only what migration currently generates, but we're not there yet.

Side issue: there is still risk of dup items in the current migration if compiler\resources\**\*.resx is non-empty (I think there was a separate issue about that).

livarcocc commented 7 years ago

From @piotrpMSFT on November 23, 2016 8:4

We'll need to discuss this offline a bit. There are a lot of parallel issues here and we need to figure out the right approach. @harshjain2 I hope you're unblocked via manual migration for the time being.

livarcocc commented 7 years ago

From @piotrpMSFT on November 23, 2016 8:5

@livarcocc FYI

livarcocc commented 7 years ago

I am not sure this is something that migration should be doing. After all, embedded resources is actually now an implicit part of the SDK and the SDK should handle the right experience.

After all, even if we do something for migrated projects, newly created projects will still not have the right behavior, if anything besides have the EmbeddedResource Include is needed.