fix: stop ignoring timer labels with a named arg, move CI to nix (#64)#3
Merged
Conversation
The `time`/`timeLog`/`timeEnd` FFI stubs took a label parameter `s` but never used it (the stub just errors), so luacheck flagged an unused argument. Rename it to `_s` to mark it deliberately unused; the stubs still fail loudly because Lua has no console-timer equivalent. Also moves CI off the dead npm/bower/setup-purescript flow onto the nix + overlay canon used by the other package-set forks: `scripts/build` under `set -euo pipefail`, luacheck `--std lua51 --no-unused-args`, and drops the broken pulp/node `scripts/test`.
There was a problem hiding this comment.
Pull request overview
This PR addresses a Lua FFI lint warning by explicitly marking the console timer label argument as unused, and migrates GitHub Actions CI from the legacy npm/bower/setup-purescript flow to a Nix-flake-based build/lint pipeline.
Changes:
- Rename the unused label parameter for
time,timeLog, andtimeEndLua FFI stubs to_sto avoid unused-arg lint warnings. - Harden
scripts/buildby enablingset -euo pipefail. - Replace the existing CI workflow with a Nix-based build/test/lint flow (including luacheck invocation).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Effect/Console.lua |
Marks the timer label arg as intentionally unused in the Lua FFI stubs. |
scripts/test |
Removes the legacy test runner script. |
scripts/build |
Enables stricter bash error handling. |
.github/workflows/ci.yml |
Switches CI to use Nix (nix develop) for build, optional test, and luacheck. |
Comments suppressed due to low confidence (1)
scripts/test:1
scripts/testis removed, but the repo still references it (e.g.package.jsonhas"test": "npm run build && ./scripts/test"). As-is, runningnpm test(or any tooling that expects this script) will fail with a missing file error. If the intention is to drop the old pulp/node tests, consider keeping a minimal placeholder script that clearly states there are no tests yet and exits successfully (or updatepackage.jsonin this PR).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CI built on a 2024-03 pslua pin that wrote --lua-output-file without creating the parent dir, so a fresh checkout failed with `dist/Effect_Console.lua: withFile: does not exist`. Current pslua ensures the output dir exists (the withOutputFile fix).
This was referenced Jun 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes purescript-lua/purescript-lua#64.
The
time,timeLogandtimeEndFFI stubs each took a label parametersand never used it (the stub just raises "not implemented", since Lua has no console-timer API), so luacheck reported an unused argument. Renaming it to_smarks it as deliberately unused. The stubs still fail loudly, which is the intended behaviour until the timers are actually implemented (tracked separately in #2).While here, this moves the fork off its dead npm/bower/setup-purescript CI onto the nix + overlay setup the other package-set forks already use:
scripts/buildruns underset -euo pipefail.nix develop, with luacheck pinned to--std lua51 --no-unused-args.scripts/testis dropped; there is no Lua-side test to run yet, and CI skips a missingscripts/test.Verification
nix develop -c ./scripts/buildcompiles the fork against the Lua package set. luacheck is clean onsrc/both with and without--no-unused-args, so the_srename fixes the warning at the source rather than only suppressing it.