Skip to content
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 suppression ability #72

Merged
merged 2 commits into from
Sep 18, 2023
Merged

Add suppression ability #72

merged 2 commits into from
Sep 18, 2023

Conversation

stan-sz
Copy link
Contributor

@stan-sz stan-sz commented Sep 14, 2023

Fixes #61

@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Merging #72 (ce8f5dd) into main (dd16a92) will increase coverage by 0.73%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #72      +/-   ##
==========================================
+ Coverage   87.32%   88.05%   +0.73%     
==========================================
  Files           3        3              
  Lines         418      452      +34     
==========================================
+ Hits          365      398      +33     
- Misses         53       54       +1     
Files Changed Coverage Δ
src/E2ETests/E2ETests.cs 91.02% <100.00%> (+0.78%) ⬆️
src/Tasks/CollectDeclaredReferencesTask.cs 87.74% <100.00%> (+0.40%) ⬆️

@erikmav
Copy link
Contributor

erikmav commented Sep 14, 2023

Needs reference documentation in README.md in the ## How to use section - perhaps:

Disabling a rule on a reference

To turn off a rule on a specific project or package reference, add the relevant RTxxxx code to a NoWarn metadata attribute. For example:

<ProjectReference Include="../Other/Project.csproj" NoWarn="RT0002" />

In reply to: 1720026036

@erikmav
Copy link
Contributor

erikmav commented Sep 14, 2023

I would also move the ## Rules section above ## How to use to introduce the RT codes earlier.


In reply to: 1720026036

Copy link
Contributor

@erikmav erikmav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@stan-sz
Copy link
Contributor Author

stan-sz commented Sep 14, 2023

I have realized that the nowarn info does not have to be passed to the analyzer. Just references with appropriate nowarn attribute will be swallowed in the msbuild task

@dfederm
Copy link
Owner

dfederm commented Sep 15, 2023

I have realized that the nowarn info does not have to be passed to the analyzer. Just references with appropriate nowarn attribute will be swallowed in the msbuild task

Yea that's what I was thinking. Just filter it on the "Declared" side.

@stan-sz
Copy link
Contributor Author

stan-sz commented Sep 15, 2023

Yea that's what I was thinking. Just filter it on the "Declared" side.

OTOH the current implementation would allow to report useless suppressions.

@stan-sz stan-sz requested a review from erikmav September 18, 2023 14:41
@dfederm dfederm merged commit e69e879 into dfederm:main Sep 18, 2023
@erikmav
Copy link
Contributor

erikmav commented Sep 19, 2023

Verified this allowed me to fix RT0002 issues in an affected repo. Found a NullRefException, sending PR for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project references used for copying binaries to output are flagged as unneeded
3 participants