fix(cli): unblock v0.8.3 release guard and make dot.li URLs TLD-aware - #264
Conversation
…aware The release guard failed the v0.8.3 CLI release because the v0.8.2..v0.8.3 range on main contained the UI package.json version fix. A release range spans both packages' history, so it is not a signal: the one-package-per-change rule is enforced at PR time, and a v* release only builds the CLI. Keep the guard's tag-equals-version check and remove the range check. Also fix dotliViewUrls, which stripped only .dot, so on a non-.dot deployment a fully-qualified name produced label.tld.tld. It now takes the resolved bare label; setContentHash returns that label.
CI Summary
Release - PassedTest this PR Download artifact (GitHub CLI required): gh run download 32242512184 -n cli-release-0.0.0-pr.264 -R paritytech/dotns-sdkInstall globally: npm install -g ./parity-dotns-cli-0.0.0-pr.264.tgzVerify: dotns --helpDeploy Example — FailedFailed at: Deploy — Deploy workflow failed — see run logs for upload/register/contenthash details
Labelspkg: cli, type: test Test - Passed301 passed, 0 failed across 301 tests. |
re-gius
left a comment
There was a problem hiding this comment.
the change breaks two existing unit tests.
packages/cli/tests/unit/cli/dotliViewUrls.test.ts was written against the old .dot-stripping behaviour and still passes fully-qualified names:
expect(dotliViewUrls("alice.dot")).toEqual(["https://alice.paseo.li"]);
expect(dotliViewUrls("alice.dot")).toEqual(["https://alice.dev-dot.li"]);Under the new signature those produce https://alice.dot.paseo.li and https://alice.dot.dev-dot.li.
Please update them and add a case that pins the regression this PR fixes, e.g. a subname:
expect(dotliViewUrls("blog.alice")).toEqual(["https://blog.alice.paseo.li"]);The old tests passed fully-qualified names (alice.dot), relying on the .dot stripping this PR removes. Use bare labels and add a subname case that pins the no-TLD-duplication fix.
|
Thanks, good catch. Those tests relied on the old |
There was a problem hiding this comment.
LGTM - thanks!
Just leave paritytech/dotns#225 open since UI fixes are missing
Description
Two fixes.
1. The release guard blocked the v0.8.3 CLI release. The
releasecheck rejected the build with:The
v0.8.2..v0.8.3range onmaincontained the UIpackage.jsonversion fix (0.7.6xc→0.7.6) that rode in with the guard PR. A release range onmainnaturally spans both packages' history, so it is not a signal. The one-package-per-change rule is enforced at PR time (checkPullRequest), and av*release only ever builds and publishes the CLI. So the release-time range check adds no safety and causes false failures. It is removed; the guard keeps the valuable tag == package.json version assertion.2.
dotliViewUrlsstripped only.dot. On a non-.dotdeployment, passing a fully-qualified name (for examplealice.paseo) producedalice.paseo.paseo.li. It now takes the resolved bare label rather than a name (the dot.li gateways already carry their own domain), andsetContentHashreturns thatlabelso the caller passes it.Type
Package
@parity/dotns-cliscripts/releaseGuard.mjs)Fixes
dotliViewUrlsduplicating the TLD on non-.dotdeployments.Testing
bun scripts/releaseGuard.mjs release --tag v0.8.3→OK: cli release v0.8.3 matches …0.8.3(previously failed).bun scripts/releaseGuard.mjs release --tag v0.8.2→ still fails on the version mismatch (guard intact).dotliViewUrls("alice")→["https://alice.paseo.li"](single TLD).cd packages/cli && bun run typecheck && bun run lint && bun test tests/unit/all pass.Notes
The
v0.8.3git tag still points at the pre-fix commit, so it must be re-cut frommainafter this merges for the CLI to publish (npm is still on0.8.2; nothing shipped). Remediation once merged:This commit is unsigned (my environment cannot reach the GPG passphrase); amend-sign if branch policy needs it.