search

giraffe-fsharp / Giraffe

A native functional ASP.NET Core web framework for F# developers.
https://giraffe.wiki
Apache License 2.0
2.13k stars 266 forks source link

Add F# Analyzers #603

Closed 1eyewonder closed 4 months ago

1eyewonder commented 4 months ago

Description

I thought it would be beneficial if we were to add some analyzers to the project in order to catch some areas we might improve. This opens up some discussion/thinking for the maintainers to consider configuration and additional pipeline opportunities.

  1. Do we want to add analyzers to the build pipeline/PR checks? See docs
  2. Are there specific analyzers we want to trigger errors/warning/ignore?

I don't have any strong opinions and just wanted to share some other cool opportunities we could try and add to this PR or others if desired.

How to test

  1. Run dotnet tool restore
  2. Run dotnet msbuild /t:AnalyzeSolution
  3. See all the suggestions image

Related issues

I was looking through some issues and came across #577 which made me think of suggesting this.

64J0 commented 4 months ago

Hello @1eyewonder, thanks for opening this PR. I'll review it during this week. For now, what I'd say is:

  1. Do we want to add analyzers to the build pipeline/PR checks?

This idea is interesting. It must be helpful for PR reviews. If you don't want to tackle it with this PR, we can do it later, no problem.

1eyewonder commented 4 months ago

If we go ahead and tackle it within this PR, I am ok with it, but I think we'll have to coordinate. I believe the GitHub Advanced Security settings needed to be configured (see docs linked above).

A repo which I've seen this feature active on is Fable

Here are some additional links to provide visual/examples to the PR reviews since that seems to be the area of interest.

64J0 commented 4 months ago

Ack, thanks for the additional material @1eyewonder! I'll try to take a proper look at this PR either this or next week.

64J0 commented 4 months ago

Cool, I think it's working:

image