-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Conversation
Feels a little more like a guide on how to call cmd line applications in FAKE. |
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. |
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. |
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. |
So here is what I was thinking for the outline of breaking the docs into multiple pages. Let me know your thoughts Getting StartedInstalling AnalyzersNuget/Paket stuff here Configuring Analyzers for Your IDEI 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 ArgsI was thinking maybe adding some docs here about required/optional command line args here if we thought that was useful Using MSBuildUsing FAKE |
Assuming each heading is its own page, I'd say this looks reasonable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great start!
docs/content/getting-started/CLI.md
Outdated
|
||
## 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. |
There was a problem hiding this comment.
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
.
docs/content/getting-started/CLI.md
Outdated
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`. |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dotnet tool install fsharp-analyzers | |
dotnet tool install fsharp-analyzers --create-manifest-if-needed |
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 |
Thanks for the latest changes, looks good now!
net8.0 .I've seen this in other projects as well. |
Thanks, @TheAngryByrd feel free to merge when this is fine for you. |
Thank you @1eyewonder ! |
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