Skip to content

Restore the public polyad() method - #284

Open
afonsojanu wants to merge 1 commit into
bgrins:masterfrom
afonsojanu:fix/restore-polyad-public-method
Open

Restore the public polyad() method#284
afonsojanu wants to merge 1 commit into
bgrins:masterfrom
afonsojanu:fix/restore-polyad-public-method

Conversation

@afonsojanu

Copy link
Copy Markdown

Fixes #281.

polyad(number) got commented out in mod.js while 1.6.0's ESM publish was still bumpy, gated on #254 as a temporary measure ("Disabled until #254"). #254 closed once 1.6.0 shipped, but the disable was never reverted and the method's test kept its ignore: true flag, so there's been no CI signal that it's missing. Calling tinycolor("red").polyad(3) on current master or the published package throws polyad is not a function, even though the internal helper it delegates to, and triad()/tetrad() which use that same helper, both work fine.

Uncommented the method and removed the ignore flag from its test, which already had the full assertion set written (throws on invalid input, correct hex output for 1 through 5 colors). Ran the local suite with deno test test.js: 46 passed, 0 failed.

One thing I didn't do: regenerate npm/dist. deno task build needs network access to a couple of external packages I couldn't reach cleanly in this environment, so I left those alone - the source-level fix is the same regardless, and presumably the build already runs as part of cutting a release.

polyad(number) was commented out back when 1.6.0's ESM publish was still
in flux, gated on issue bgrins#254 as a temporary measure. That issue closed
once 1.6.0 shipped, but nobody went back and uncommented the method or
removed the ignore flag on its test, so it's been silently unreachable
on master and in the published package ever since. Calling
tinycolor("red").polyad(3) throws "polyad is not a function" even
though the internal helper backing it, and the triad()/tetrad() methods
built on top of the same helper, all work fine.

Uncommented the method in mod.js and dropped the ignore: true from its
Deno test, which already had the full 9-assertion spec written and
waiting. Ran the full local suite (deno test test.js): 46 passed, 0
failed, including all the polyad assertions. Didn't touch the generated
npm/dist bundles since those come out of the repo's own build step
(deno task build), which needs network access to a couple of external
packages that isn't available here to reproduce cleanly - the source
change is the same either way, and running the build is presumably
already part of the release process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

polyad() is missing from master and published 1.6.0 — the temporary disable (issue #254) was never reverted

1 participant