tunnelvisionlabs / ReferenceAssemblyAnnotator

IL weaver to add nullability annotations to .NET reference assemblies
MIT License
72 stars 8 forks source link

No-op on target frameworks that are already annotated #49

Closed jnm2 closed 4 years ago

jnm2 commented 4 years ago

Superseded by https://github.com/tunnelvisionlabs/ReferenceAssemblyAnnotator/pull/59

Also fixes #38.

Tested on my Shouldly branch with two target frameworks temporarily appended:

<TargetFrameworks>net451;netstandard1.3;netstandard2.0;netstandard2.1;netcoreapp3.0</TargetFrameworks>

Before, #38 was reproing. After, everything was as expected (annotation on only the first three).

sharwell commented 4 years ago

The main issue here is we don't actually want to no-op, because we still need to rewrite the reference assemblies to drop the attributes from #46. The removal itself is already implemented:

https://github.com/tunnelvisionlabs/ReferenceAssemblyAnnotator/blob/ea0490148d3c8b158b68ede5e81f1ccf012175f5/TunnelVisionLabs.ReferenceAssemblyAnnotator/Program.cs#L174-L182

Are the netstandard2.1 reference assemblies annotated? I was expecting them to have the attribute in place, but not have the nullable annotations. Could be wrong though.

jnm2 commented 4 years ago

The main issue here is we don't actually want to no-op, because we still need to rewrite the reference assemblies to drop the attributes from #46.

I am very scared. This is kind of like a hard fork then for anyone who uses this package. I'll do it if I have to but I really don't like it. My understanding was that this was only for when you can't use HashCode.

jnm2 commented 4 years ago

Are the netstandard2.1 reference assemblies annotated? I was expecting them to have the attribute in place, but not have the nullable annotations. Could be wrong though.

Just tried it. They are not annotated, good catch.

jnm2 commented 4 years ago

Closing because no_op is not a good branch name for the work that needs to be done.

jnm2 commented 4 years ago

we still need to rewrite the reference assemblies to drop the attributes

What if there's a csproj flag for this?