Fake build project and GitHub actions per scenario by xperiandri · Pull Request #739 · fsprojects/FSharpLint

xperiandri · 2025-07-07T23:33:24Z

Build project on .NET 9
launchSettings.json with multiple profiles to run the build project
Target to execute locally and issue a release: update changelog, commit, set tag
Publish successful merges to master as prerelease GitHub packages
Embed all sources into GitHub packages for easy local debugging
GitHub actions for CI, merge to master and release tag creation

Copilot

Pull Request Overview

This PR introduces a new FAKE-based build project, replaces the old build.fsx script, adds AltCover support in tests, and configures multiple GitHub Actions workflows.

Add AltCover package to test projects for code coverage
Create build/build.fsproj and build/build.fs as the new FAKE build setup, with accompanying wrappers (build.sh/build.cmd)
Remove legacy build.fsx, update solution files, central package versions, and add CI/CD GitHub workflows

Reviewed Changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests//.fsproj	Added `<PackageReference Include="altcover" />` to enable coverage instrumentation
build/build.fsproj	New FAKE build project with dependencies
build/build.fs	New F# script implementing build targets
build.sh / build.cmd	Shell and Windows wrappers for invoking FAKE build
build.fsx	Removed legacy build script
Directory.Packages.props	Reorganized package version groups
.github/workflows/*.yml	Added multiple workflows for build, publish, and docs deployment

Comments suppressed due to low confidence (1)

build/build.fs:92

The changelogBackupFilename mutable value is declared but never used within this script. Consider removing it to reduce dead code.

let mutable changelogBackupFilename = ""

Copilot · 2025-07-07T23:34:46Z

+    |> Seq.map (fun t -> t.Name)
+    |> Seq.exists ((=) "PublishToNuGet")
+
+let invokeAsync f = async { f () }


Prefer using the task computation expression over async here (e.g., let invokeAsync f = task { f () }) to align with the project’s guidelines.

Disagree, Copilot. async is idiomatic F#, task is not (it was created for better interoperability with C#)

It is according to the Copilot instructions file.
https://github.com/fsprojects/FSharpLint/pull/726/files
If you want to suggest any changes in that PR I will review

xperiandri · 2025-07-07T23:58:53Z

@knocte @Numpsy @webwarrior-ws I've updated CI to cover more scenarios. Give your feedback, please.
Taken from https://www.jimmybyrd.me/MiniScaffold

knocte · 2025-07-08T04:25:54Z

@@ -0,0 +1,73 @@
+name: Build `master`


@xperiandri no need to rename the workflow file because this still builds & run tests. Also no need to rename workflow name because PRs should run CI too (and PRs are not 'master')

But the name was build+test+deploy.yml and now it does not deploy anything. Only builds and tests. Do you still want to have the same name?

having it same name would, to begin with, allow to have a proper diff to comment on; and I'm not convinced about moving the deploy parts of the CI to another file because you're basically moving logic from F# to GithubActions syntax, so basically undoing some work we have done in the past

The main purpose of having multiple pipelines is the separation of responsibility. You always know which file responsible for what.
Also, I reconsolidated Fornax build logic into the Fake build project. So that the docs build is encapsulated into a build target.
@webwarrior-ws, what do you think?

…efloader.fsx`

xperiandri · 2025-07-08T20:25:11Z

-    - name: Setup .NET
-      uses: actions/setup-dotnet@v4
-      with:
-        dotnet-version: ${{ env.DOTNET_VERSION }}

-    - name: Restore tools
-      run: dotnet tool restore
-    - name: Restore dependencies
-      run: dotnet restore
-    - name: Run Fornax
-      run: dotnet fsi build.fsx -t Docs
-    - name: Deploy (if tag)
-      if: startsWith(github.ref, 'refs/tags/')
-      uses: peaceiris/actions-gh-pages@v3


Now all of this is done in Fake via a designated target

…n `apirefloader.fsx`

knocte · 2025-07-09T06:31:15Z

+      - master
+  pull_request:
+    branches:
+      - master


@xperiandri doing the above will make PRs not have CI feedback; please revert all these changes in the workflow above

knocte · 2025-07-09T06:32:01Z

-        - windows-latest
-        - macOS-latest
+        configuration: [Debug, Release]
+        os: [ubuntu-latest, windows-latest, macOS-latest]


@xperiandri there's no need to change the os attribute for no reason, you're just causing git-blame line-damage here

knocte · 2025-07-09T06:32:47Z

+    - name: Build via Bash
+      if: runner.os != 'Windows'
+      run: |
+        chmod +x ./build.sh


@xperiandri let's just add the +x attrib to the file in github to not need to use chmod here

knocte · 2025-07-09T06:33:56Z

-        publish_branch: gh-pages
-        force_orphan: true
+        runCmd: |
+          chmod +x ./build.sh


@xperiandri same here

knocte · 2025-07-09T06:34:51Z

+on:
+  # Runs on pushes targeting the default branch
+  push:
+    branches: ["master"]


@xperiandri we want PRs to the docs to also be tested before being merged, so please stop using the branches attribute, it is a bad practice

knocte · 2025-07-09T06:35:14Z

+
+      - name: Build Docs
+        run: |
+          chmod +x ./build.sh


@xperiandri same here about chmod

knocte · 2025-07-09T06:37:51Z

+on:
+  push:
+    branches:
+      - master


@xperiandri even if we only want to publish for master branch, the way to filter it is to prevent the last step from happening (the nuget push) if branch is not master. This way the job is still being tested everytime, even for PRs.

knocte · 2025-07-09T06:38:42Z

+      - name: Add the GitHub source
+        run: dotnet nuget add source --name "github.com" "https://nuget.pkg.github.com/fsprojects/index.json"
+
+      - name: Ensure NuGet package source mapping


@xperiandri what is this exactly? I would prefer a script instead of so much githubActions logic

moved to build.fs

knocte · 2025-07-09T06:40:01Z

+on:
+  push:
+    tags:
+      - 'v*'


@xperiandri same point I mentioned about filtering to master branch applies here

knocte · 2025-07-09T06:42:33Z

@@ -0,0 +1 @@
+dotnet run --project ./build/build.fsproj -- -t %*


@xperiandri I guess -t means --target? let's prefer to use longer flag names, shorter ones can be used when using the terminal to type faster, but for scripts we should favour readability

knocte · 2025-07-09T06:42:51Z

+set -eu
+set -o pipefail
+
+FAKE_DETAILED_ERRORS=true dotnet run --project ./build/build.fsproj -- -t "$@"


@xperiandri same here about -t

…m YAML to Fake

xperiandri · 2025-07-09T23:06:06Z

@knocte, let's define the build strategy.
With the current setup build happens twice (when a branch is in this repo), and every time it passes the same stages.
The approach proposed in the PR is:

CI when PR to master
CD publish NuGet with embedded PDBs to GitHub when any commit appears in master
CD publish to NuGet when tag appears

You suggest:

Do CI not only for PRs to master but for any branch. I agree.
Perform the Docs CI, not only CD.

I propose:

I agree and change
We use a stage in the CI pipeline for docs, and a stage in the NuGet CD pipeline. Removing the separate docs pipeline.

xperiandri · 2025-07-09T23:10:59Z

Looks like a bug. It treats named parameter assignment as a condition

========== Linting /home/runner/work/FSharpLint/FSharpLint/tests/FSharpLint.FunctionalTest/TestApi.fs ==========
`x = true` might be able to be refactored into `x`.
Error on line 22 starting at column 47
            let checker = FSharpChecker.Create(keepAssemblyContents=true)
                                               ^                         
See https://fsprojects.github.io/FSharpLint/how-tos/rules/FL0065.html

knocte · 2025-07-10T08:22:30Z

Looks like a bug

It is.

knocte · 2025-07-10T08:25:09Z

-    tags:
-      - 'v*'
+    branches:
+      - master


@xperiandri I know that you have put '**' for pull_request, but still putting this filter here will mean that a contributor that is about to post a PR won't get CI feedback until she proposes the PR, which is unnecessary. Contributors should be able to see the CI statuses before creating PRs.

knocte · 2025-07-10T08:26:57Z

-
-  # to execute once a day (more info see https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule )
-  schedule:
-    - cron: '0 0 * * *'


@xperiandri why remove the schedule? this way we make sure that CI is still working, and when it breaks for reasons unrelated to FSharpLint commits we notice right away. Before I added this schedule job it used to happen that suddenly some new commit landed in master and broke CI and we were scratching our heads for days wondering how that commit could break the build, until we realised the CI broke by itself, unrelated to the commit.

knocte · 2025-07-10T09:19:51Z

let's define the build strategy.

Not sure I understand what you're getting at in that big comment, Take in account there's already a build strategy in master branch so what you're trying to do in a PR is to change it. If you phrase the changes this way, I'll understand much better what you mean. For example, wrt the docs, you mean we shouldn't run fornax in master but only in tags? If that's the case, say it explicitly: stop doing X, start doing Y, etc.

Numpsy · 2025-07-10T10:10:18Z

Looks like a bug. It treats named parameter assignment as a condition

========== Linting /home/runner/work/FSharpLint/FSharpLint/tests/FSharpLint.FunctionalTest/TestApi.fs ==========
`x = true` might be able to be refactored into `x`.
Error on line 22 starting at column 47
            let checker = FSharpChecker.Create(keepAssemblyContents=true)
                                               ^                         
See https://fsprojects.github.io/FSharpLint/how-tos/rules/FL0065.html

#492

xperiandri · 2025-07-11T09:57:08Z

Migrated to #742

Copilot AI review requested due to automatic review settings July 7, 2025 23:33

Copilot AI reviewed Jul 7, 2025

View reviewed changes

xperiandri changed the title ~~Fake build project and mutiple GitHub actions~~ Fake build project and GitHub actions per scenario Jul 7, 2025

knocte reviewed Jul 8, 2025

View reviewed changes

Comment thread tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj Outdated

knocte reviewed Jul 8, 2025

View reviewed changes

Comment thread build/build.fsproj Outdated

knocte reviewed Jul 8, 2025

View reviewed changes

Comment thread .devcontainer/devcontainer.json Outdated

xperiandri added 3 commits July 8, 2025 17:44

refactor(docs): migrate Fornax to 0.15.1

58c72ef

fix(docs): reference resolution and missing site info

5e5b4d0

feat(ci): signal the pipeline that docs generation failed

2d6a6ec

xperiandri force-pushed the fornax-0.15.1 branch from 8bd77ad to 2d6a6ec Compare July 8, 2025 13:46

xperiandri added 2 commits July 8, 2025 18:28

refactor(docs): simplify generateNamespace funtions usage

5ff152c

refactor(docs): use variable for the common string constants in `apir…

1d3b137

…efloader.fsx`

xperiandri force-pushed the release-automation branch 2 times, most recently from fa526b5 to c33ab51 Compare July 8, 2025 19:49

xperiandri commented Jul 8, 2025

View reviewed changes

xperiandri added 2 commits July 9, 2025 00:49

fixup! feat(ci): signal the pipeline that docs generation failed

610075f

fixup! refactor(docs): use variable for the common string constants i…

0c501ed

…n `apirefloader.fsx`

knocte reviewed Jul 9, 2025

View reviewed changes

xperiandri added 18 commits July 10, 2025 02:19

fixup! fix(docs): reference resolution and missing site info

7f487c2

fixup! Fornax 0.16.0 and FSharp.Formatting 20.0.1

bf95a0b

fixup! Fornax 0.16.0 and FSharp.Formatting 20.0.1

53634ab

feat(ci): Fake build project

96cee24

feat(ci): run all tests on new CI

1273264

feat(ci): run Fornax to build docs

2d0b69d

feat(ci): build only FSharpLint.Core for docs

3e086ee

feat(ci): self check the solution

6ed6390

feat(ci): use v* version tag instead of releases/*

bb29921

fixup! feat(ci): Fake build project

f03ba05

fixup! feat(ci): Fake build project

a66df45

squash! feat(ci): Fake build project

f51c133

chore(ci): remove Octokit package version update

351d768

fixup! feat(ci): Fake build project

79fac28

fixup! feat(ci): self check the solution

16ef629

fixup! feat(ci): Fake build project

450c033

refactor(ci): move NuGet.config package source mapping creation fro…

7778841

…m YAML to Fake

refactor(ci): parse NuGet.config with System.Xml.Linq

15505f9

xperiandri force-pushed the release-automation branch from cd55eca to 15505f9 Compare July 9, 2025 22:51

fixup! feat(ci): Fake build project

5e2c1a3

knocte reviewed Jul 10, 2025

View reviewed changes

xperiandri force-pushed the fornax-0.15.1 branch from 53634ab to b173d9f Compare July 10, 2025 12:40

xperiandri closed this Jul 11, 2025

		@@ -0,0 +1 @@
		dotnet run --project ./build/build.fsproj -- -t %*

Conversation

xperiandri commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xperiandri commented Jul 7, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xperiandri commented Jul 9, 2025

Uh oh!

xperiandri commented Jul 9, 2025

Uh oh!

knocte commented Jul 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xperiandri commented Jul 7, 2025 •

edited

Loading