Skip to content

Fix opam package URLs, the lint-doc message, and the release script - #1137

Merged
smorimoto merged 8 commits into
masterfrom
fix-small-defects
Oct 18, 2026
Merged

smorimoto merged 8 commits into
masterfrom
fix-small-defects

Conversation

@smorimoto

@smorimoto smorimoto commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Why

A review of the code base found several small defects in the actions and the release tooling. Each commit fixes one of them and stands alone. The first commit adds expect tests, so the package URL fix shows up as a one-line snapshot change.

What changed

  • The analysis action no longer encodes opam package names twice. conf-g++ now becomes pkg:opam/conf-g%2B%2B@1.0 instead of pkg:opam/conf-g%252B%252B@1.0.
  • lint-doc reports its own Odoc message instead of the generic exit-code annotation, and the cache code stops retrying errors that a retry cannot fix.
  • scripts/release.sh moves the major version tag in one atomic push and stops at the first error, so @v3 always resolves during a release.

Scope

  • Also removes no-op code, the README copies inside packages/, and .gitattributes rules for files that the build no longer writes.
  • Adds Vitest through vite-plus, a yarn test script, and a test step in the hygiene job.

Blast radius

The dependency graph lists packages that have + in their names under new, correct identifiers. The release script change affects maintainers only.

Verification

  • yarn lint, yarn knip, yarn typecheck, CI=true yarn test, and a clean yarn build pass on every commit.
  • The built lint-doc bundle, run against a fake opam that fails, now annotates the step with the Odoc message.
  • release.sh, run against a throwaway bare remote, moves the major tag atomically and completes a rerun after a failed push.

Run Vitest through vite-plus in every package that defines a test
task, and run it in the hygiene job. Tests print their results as
text and compare it with an inline snapshot, in the manner of Jane
Street's expect tests. "yarn test:update" rewrites the snapshots in
the test source, and the diff is reviewed like any other change.
Strings are printed as they are, without quotes, so a snapshot reads
as the output it records.

The first test records the package URLs that the analysis action
submits. The snapshot shows today's output, in which a name that
contains "+" is encoded twice.
PackageURL already percent-encodes the name when it serialises the
URL, so passing an encodeURIComponent result turned "conf-g++" into
"conf-g%252B%252B". The dependency graph therefore listed such
packages under identifiers that match nothing.
@actions/exec rejects on a non-zero exit status unless
ignoreReturnCode is set, so the exit code check never ran and users
saw the generic "failed with exit code 1" annotation instead. Also
drop the PATH override, which copied the variable onto itself.
The analysis action passed process.env with PATH copied onto itself,
which is what @actions/exec does without an env option. lint-fmt
normalised line endings in a one-caller module before splitting on
"\n"; splitting on any line ending does the same in place.
@actions/cache handles transport failures itself. restoreCache and
saveCache log them and resolve. What still escapes is deterministic,
such as a ValidationError for the key we pass, an empty path list, or
a missing runtime token. The exponential back-off therefore never
retried a network failure. It only repeated those errors five times,
after which a notice blamed the GitHub cache backend. Report the
actual error as a warning instead, whatever value was thrown.
The packages are private and never published, so these files were
unread copies of the READMEs at the repository root and in each
action directory. They had already drifted: the lint-doc copy was the
lint-opam README.
The *.txt rules covered the licence files that the @vercel/ncc build
wrote next to the bundles. Those files went away with ncc, and the
vite-plus build writes only .mjs files.
The script deleted the remote major tag and pushed its replacement
in a later step, so for that interval "ocaml/setup-ocaml@v3" did not
resolve. It also kept going after a failed command, so a failed push
still created a GitHub release, and "git push --tags" pushed every
local tag. Fail on the first error, and force-update only the major
tag alongside the new full tag in a single atomic push. A rerun after
a failed push reuses the full tag when it already points at HEAD.
@smorimoto
smorimoto added this pull request to stack #1142 October 18, 2026 15:39
@smorimoto smorimoto changed the title fix small defects Fix opam package URLs, the lint-doc message, and the release script Oct 18, 2026
@smorimoto smorimoto added the enhancement New feature or request label Oct 18, 2026
@smorimoto
smorimoto merged commit 72a1307 into master Oct 18, 2026
19 checks passed
@smorimoto
smorimoto deleted the fix-small-defects branch October 18, 2026 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant