Skip to content

Conversation

@FlashOnFire
Copy link

@FlashOnFire FlashOnFire commented Nov 1, 2025

Follow up PR after #349

Added CI workflow that triggers when a version change is pushed (in nixfmt.cabal) to the master branch.
It automatically builds a static binary, tags the commit, and creates a GitHub release with notes pulled from CHANGELOG.md.

Example release: https://github.com/FlashOnFire/nixfmt/releases/tag/v4.1.0

Edit:
Just thought that, as an alternative, we could have a workflow that triggers when a release is published and simply appends the static binary to it. That way we can keep the release process as-is.
Let me know which option you prefer.

@github-actions
Copy link

github-actions bot commented Nov 1, 2025

Nixpkgs diff

Copy link
Collaborator

@jfly jfly left a comment

Choose a reason for hiding this comment

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

Love this, thank you! Left a bunch of nits inline (I learned a bunch about github actions thanks to @infinisil and @MattSturgeon).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a default shell as done in https://github.com/NixOS/nixpkgs/blob/7a7849f5f075f6cfa8d6607c65048b7a22479bcb/.github/workflows/build.yml#L21-L23. A nice side effect of this is to get some sane shell behavior (set -euo pipefail or something like that).

echo "Version found in cabal: $VERSION"

# Get previous version from last commit
PREV_VERSION=$(git show HEAD~1:"$CABAL_FILE" | grep -m1 "^version:" | awk '{print $2}' || echo "none")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm worried about us failing to parse the file and then strange things happening. Could we remove the || ... part?

Suggested change
PREV_VERSION=$(git show HEAD~1:"$CABAL_FILE" | grep -m1 "^version:" | awk '{print $2}' || echo "none")
PREV_VERSION=$(git show HEAD~1:"$CABAL_FILE" | grep -m1 "^version:" | awk '{print $2}')

Comment on lines +39 to +40
run: |
if [ "${{ steps.get_version.outputs.version }}" != "${{ steps.get_version.outputs.prev_version }}" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's important to pass inputs via env vars instead of directly interpolating into the shell script. Here's how:

Suggested change
run: |
if [ "${{ steps.get_version.outputs.version }}" != "${{ steps.get_version.outputs.prev_version }}" ]; then
env:
PREV_VERSION: ${{ steps.get_version.outputs.prev_version }}
VERSION: ${{ steps.get_version.outputs.version }}
run: |
if [ "$PREV_VERSION" != "$VERSION" ]; then

Similar other interpolations below.

run: |
if [ "${{ steps.get_version.outputs.version }}" != "${{ steps.get_version.outputs.prev_version }}" ]; then
echo "version_changed=true" >> $GITHUB_ENV
echo "::notice ::✅ Version changed from ${{ steps.get_version.outputs.prev_version }} to ${{ steps.get_version.outputs.version }}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Indentation is a bit wonky here (and in the else block). Please fix.

echo "(Make sure you have a header like '## $VERSION -- YYYY-MM-DD')"
echo "- No notes will be attached to the release -"
echo "No changelog section found for version $VERSION." > RELEASE_NOTES.md
fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's explicitly exit nonzero:

Suggested change
fi
exit 1
fi

- name: Build static binary
if: env.version_changed == 'true'
run: |
nix build -L .#nixfmt-static
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use stable nix here instead of flakes:

Suggested change
nix build -L .#nixfmt-static
nix-build -A packages.nixfmt-static

if: env.version_changed == 'true'
run: |
nix build -L .#nixfmt-static
mkdir -p release
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we use this release directory? Can we remove this line?

tag_name: "v${{ steps.get_version.outputs.version }}"
name: "v${{ steps.get_version.outputs.version }}"
body_path: RELEASE_NOTES.md
files: "${{ env.artifact_path }}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's just inline artifact_path:

Suggested change
files: "${{ env.artifact_path }}"
files: ./result/bin/nixfmt

git config user.name "Github Actions"
git config user.email "[email protected]"
git tag -a "$TAG" -m "Release $TAG"
git push origin "$TAG"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use the gh cli directly (it's installed by default). Take a look at gh release create --help (it will likely create the tag for us).

@jfly jfly moved this to Approved in Nix formatting Nov 11, 2025
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/formatting-team-meeting-2025-11-11/72019/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Approved

Development

Successfully merging this pull request may close these issues.

3 participants