Skip to content

Conversation

@Corniel
Copy link
Contributor

@Corniel Corniel commented Feb 26, 2025

Documentation for issue 376. I'm not sure yet what title to give it. Any suggestions?

@Corniel Corniel added the documentation Improvements or additions to documentation label Feb 26, 2025
@Corniel Corniel self-assigned this Feb 26, 2025
@CptWesley
Copy link
Member

Title suggestion: Remove redundant None nodes

Exceptions should perhaps be made for cases where the None node does do something (e.g. specify a custom path) that the other node is not doing (although this should probably fire off some other warning for being illegible and I'm not 100% sure how IDEs handle this situation).

@Corniel
Copy link
Contributor Author

Corniel commented Feb 26, 2025

Title suggestion: Remove redundant None nodes

Partly I like the name, but it suggest way more than the rule covers (right now). We're not checking Include or Update <None>s that are not needed (which also can be the case). Do we, once we have a way to detect them, in the future put that under the same rule?!

Exceptions should perhaps be made for cases where the None node does do something (e.g. specify a custom path) that the other node is not doing (although this should probably fire off some other warning for being illegible and I'm not 100% sure how IDEs handle this situation).

<ItemGroup>
  <None Include="*.md">
    <CopyToOutputDirectory>Always</CopyToOutputDirectory>
  </None>
  <None Remove="LICENSE.md" />

But this can also be written as:

<ItemGroup>
  <None Include="*.md">
    <CopyToOutputDirectory>Always</CopyToOutputDirectory>
  </None>
  <None Update="LICENSE.md">
    <CopyToOutputDirectory>Never</CopyToOutputDirectory>
 </None>

And yes, this is longer, but it is an - at least I think - uncommon scenario, that is preferable because of the simplicity of the implementation which leads to less FN's, and the unlikelyness of FP's.

@Corniel
Copy link
Contributor Author

Corniel commented Feb 26, 2025

@Gemberkoekje Do you have a preferred title in mind?

@Gemberkoekje
Copy link
Contributor

Gemberkoekje commented Feb 26, 2025

Minimize the usage of NONE ?

@LauraXiulan
Copy link
Contributor

LauraXiulan commented Feb 26, 2025

"Remove redundant None nodes with Remove attribute"? I like Wesleys suggestion, we could expand the rule later.

Another consideration, what to do when attributes are combined in one statement. For example this is possible:

<ItemGroup>
  <!-- Include every C# source file, except anything in the "sub" folder -->
  <Compile Include="**/*.cs" Exclude="sub/**/*.cs">
</ItemGroup>

Is that possible for None Remove as well? Although the initial issue was raised based on 'accidental' adding and this definitely is an explicit action. So might be out of scope.

@Corniel Corniel changed the title Proj0036: Usage of None Remove Proj0036: Remove None when redundant Feb 26, 2025
@Corniel Corniel marked this pull request as ready for review February 26, 2025 13:49
@Corniel
Copy link
Contributor Author

Corniel commented Feb 26, 2025

Based on rule Proj0026, I went for: Remove None when redundant.

Co-authored-by: Laura Kramer <[email protected]>
Co-authored-by: Wesley Baartman <[email protected]>
@Corniel Corniel merged commit 26668c3 into main Feb 26, 2025
2 checks passed
@Corniel Corniel deleted the proj0036 branch February 26, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants