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

Introduce formatting and analyzers #36

Merged
merged 8 commits into from
Jan 31, 2024
Merged

Introduce formatting and analyzers #36

merged 8 commits into from
Jan 31, 2024

Conversation

kfcampbell
Copy link
Member

This project already has an .editorconfig courtesty of #15. This PR adds a CI check and a script that can be used locally, both of which call dotnet format to ensure our source files are compliant with the .editorconfig.

Copy link

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@kfcampbell kfcampbell changed the title Introduce linting Introduce formatting Jan 30, 2024
@kfcampbell
Copy link
Member Author

Hmm...I had set this up to use https://github.com/dotnet/code-analysis, but that repository hasn't been updated in 3 years and does not run on .NET 8. I believe it to be abandoned and I don't want to run on a super old version of .NET just to use an old analysis tool.

Instead, I'll focus on using the built-in Roslyn analyzers.

@kfcampbell
Copy link
Member Author

I've implemented the following snippet in our .csproj files:

    <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
    <EnableNETAnalyzers>true</EnableNETAnalyzers>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
    <!-- The below NoWarn is due to Kiota generating code marked as Obsolete.
      This is expected, but we still want to error on other warnings from analyzers. -->
    <NoWarn>$(NoWarn);CS0618</NoWarn>

This explicltly enables .NET analyzers (though they're on by default because we use .NET 8) and treats warnings as errors. I've ignored all CS0618 "Obsolete" errors since Kiota expectedly generates some code with "Obsolete" modifiers.

Together with our CodeQL analysis Action, this should cover all necessary linting pieces for our dotnet-sdk.

@kfcampbell kfcampbell marked this pull request as ready for review January 30, 2024 22:55
@kfcampbell kfcampbell changed the title Introduce formatting Introduce formatting and analyzers Jan 30, 2024
@nickfloyd nickfloyd merged commit 24f96a2 into main Jan 31, 2024
3 checks passed
@nickfloyd nickfloyd deleted the linting branch January 31, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants