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

Use EditorConfig for dotnet build and suppress existing issues #1676

Merged
merged 1 commit into from Jan 24, 2022

Conversation

JustinGrote
Copy link
Collaborator

@JustinGrote JustinGrote commented Jan 24, 2022

This PR will configure the various .NET projects to use the editorconfig suppression settings, and adds several to demote existing (but working) warnings to suggestions, including rosylnator.

This is just meant as a stop-gap to ensure that dotnet build in CI looks "clean", so that finding/inspecting errors takes less time. Next step is to go back and selectively remove the warnings and either fix the code or drop a selective exception in for that particular warning. "Net-New" warning categories not already present in the current code have not been demoted and will continue to be reported as such.

With this, the VSCode is no longer a sea of yellow warnings, so when working on a PR it's much easier to see where specifically your code has issues. All issues are still reported as suggestions however (currently 1273 total...) but that can be easily toggled as a filter in VSCode Problems pane.
image

Instead of using `silence` we now use `suggestion` which enables
Roslynator and other tools to continue to show the warning, but not
block the build. This allows us to set `EnforceCodeStyleInBuild` now,
without having to fix everything, which helps prevent future mistakes.

Note that many of these should be fixed ASAP, and that we previously
thought we'd enabled all the maintainability warnings, but since they
are not turned on by default, the category does nothing.

Co-authored-by: Andy Schwartzmeyer <andrew@schwartzmeyer.com>
@andyleejordan
Copy link
Member

Woo! Thanks for this @JustinGrote. I added documentation to all the suggestions added, and used the common.props file to set the enforcement (instead of each file individually). I double checked that all the files I reverted did indeed include common.props and resolved the TODO in that file.

@JustinGrote
Copy link
Collaborator Author

Woo! Thanks for this @JustinGrote. I added documentation to all the suggestions added, and used the common.props file to set the enforcement (instead of each file individually). I double checked that all the files I reverted did indeed include common.props and resolved the TODO in that file.

Sounds good! Forgot about common.props

@andyleejordan andyleejordan enabled auto-merge (squash) January 24, 2022 18:12
Copy link
Member

@andyleejordan andyleejordan left a comment

Choose a reason for hiding this comment

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

🥳

@andyleejordan andyleejordan merged commit 47ce69f into master Jan 24, 2022
@andyleejordan andyleejordan deleted the suppressExistingIssues branch January 24, 2022 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants