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

Update Getting Started Docs: Calling Analyzers w/ FAKE #225

Merged
merged 5 commits into from
Jan 28, 2025

Conversation

1eyewonder
Copy link
Contributor

This PR is a documentation suggestion on calling F# analyzers from FAKE if MSBuild is not the desired workflow a developer wants to take in their repo. Let me know if this is desired or not and if I can make any updates which we think read and/or navigate easier.

An example of this working in the wild can be found here in FsToolkit.ErrorHandling

@nojaf
Copy link
Contributor

nojaf commented Jan 14, 2025

Feels a little more like a guide on how to call cmd line applications in FAKE.
There is nothing really analyzer specific about any of this.

@1eyewonder
Copy link
Contributor Author

Feels a little more like a guide on how to call cmd line applications in FAKE. There is nothing really analyzer specific about any of this.

I don't disagree, however when reading through the docs I do think people can be caught up in the mindset that the analyzers will only work with MSBuild. I think it would be beneficial, especially for ease of adoption, to provide example(s) of alternative implementations which may align with what users may already have set up.

@voronoipotato
Copy link

I didn't use this because the msbuild changes seemed confusing and made me afraid that I might make a mistake and mess up the build, @1eyewonder showed me that I could use it with FAKE and I told him he should add something to the documentation here. This approach is a lot less magical to me.

@nojaf
Copy link
Contributor

nojaf commented Jan 14, 2025

We might consider splitting our current "Getting Started" page into multiple pages.

First, we could create a dedicated page for all things related to MSBuild.

Additionally, it would be beneficial to have a page focused on Paket, as it is the most interesting aspect. The specific implementation, whether it's through FAKE or Fun.Build, may not be as important to discuss.

@1eyewonder
Copy link
Contributor Author

So here is what I was thinking for the outline of breaking the docs into multiple pages. Let me know your thoughts

Getting Started

Installing Analyzers

Nuget/Paket stuff here

Configuring Analyzers for Your IDE

I figured it might be more clear if we were to add how to setup up analyzers for VSCode here rather than have users read through an old blog post

Analyzer Command Line Args

I was thinking maybe adding some docs here about required/optional command line args here if we thought that was useful

Using MSBuild

Using FAKE

@nojaf
Copy link
Contributor

nojaf commented Jan 20, 2025

Assuming each heading is its own page, I'd say this looks reasonable.
Command line args is a tricky one, I would prefer a better --help if what we have today is insufficient.

Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

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

Great start!


## Example Command

When running the CLI tool from the command line, the bare minimum you need to provide is the path to the project file(s) you want to analyze.
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be a bit misleading. It sounds like something will happen simply by downloading the tool and pointing it to a project, but you actually need to provide an analyzer.

The default argument will only have an effect if you:

  • Use Paket in the old mode with that exact default folder name.
  • Already have a compatible analyzer.

This scenario seems quite unlikely at this point. It might be better to mention later that if you omit the --analyzers-path, it defaults to packages/analyzers.

dotnet fsharp-analyzers --project ./YourProject.fsproj --analyzers-path ./path/to/analyzers/directory
```

⚠️ If you don't provide this argument, it will default to `packages/analyzers`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is really a Paket thing and should be mentioned as such. Folks using NuGet might not get how you can ever download the analyzers in that folder.


## Visual Studio Code

In order to configure analyzers for VSCode, you will need to update your project's `.vscode/settings.json` file or your user settings. You should only need the following settings:
Copy link
Contributor

Choose a reason for hiding this comment

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

@TheAngryByrd, where can people find the log if there's an SDK mismatch? Users may use older analyzers, which might not be detected by the tooling. I believe we mention this somewhere. I suggest including an explanation on this page about where this information will be displayed.

Copy link
Member

Choose a reason for hiding this comment

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

Briefly looking at the code, I'm not sure we actually log this correctly.

Added issue here: ionide/FsAutoComplete#1350

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright, thanks, @1eyewonder maybe we just link to that issue in the docs.

A dotnet CLI tool, called [fsharp-analyzers](https://github.com/ionide/FSharp.Analyzers.SDK/), is used to run analyzers outside the context of an IDE. Add it to your tool-manifest with:

```shell
dotnet tool install fsharp-analyzers
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dotnet tool install fsharp-analyzers
dotnet tool install fsharp-analyzers --create-manifest-if-needed

@1eyewonder
Copy link
Contributor Author

Pushed the latest doc changes. Sorry I've been away for a bit. Question regarding the CI build. Is this something that should be breaking if I didn't change anything? I can try to fix this if needed

@nojaf
Copy link
Contributor

nojaf commented Jan 28, 2025

Thanks for the latest changes, looks good now!
As for the CI, the problem is not related to your changes but could you change

to net8.0.
I've seen this in other projects as well.

@nojaf
Copy link
Contributor

nojaf commented Jan 28, 2025

Thanks, @TheAngryByrd feel free to merge when this is fine for you.

@TheAngryByrd TheAngryByrd merged commit 9a294eb into ionide:main Jan 28, 2025
2 checks passed
@TheAngryByrd
Copy link
Member

Thank you @1eyewonder !

@1eyewonder 1eyewonder deleted the using-fake-docs branch January 28, 2025 16:09
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.

4 participants