Repository navigation
fix: repair the build pipeline, add CI, ship 1.4.1 - #78
Merged
Merged
Conversation
The generators reported success on failure. Writes inside forEach were never awaited and neither entry point had an error handler, so a failed write still exited 0. A CI job trusting that exit code would publish corrupt output. Both generators now await every write and exit 1 on error, verified by injecting a failing write and observing exit 1. The iTerm2 generator had never run at all: xmlbuilder was imported but never declared, so the step crashed on every invocation and the failure was hidden by the same bug. It now runs, and reproduces the four committed .itermcolors files byte for byte. vsce and ovsx were called by the publish scripts without being declared, so publishing only worked when they happened to be installed globally. .vscodeignore excluded almost nothing, so the published package carried screenshots/, src/, demos/, and node_modules: 7 MB for a theme that is 57 KB of JSON. vsce also refused to package at all because demos/.env matched its secret check. Both are fixed by the ignore list. The README is unaffected since every image it references is a remote URL. Bump to 1.4.1 and write a real changelog. The previous one said "Initial release" while ten variants were live. No theme JSON is touched by this commit.
Closes #2, open since December 2022. Publishing has been manual, which is how a fix merged in November 2024 sat unreleased for fifteen months. ci.yml runs on every PR: typecheck, regenerate, drift gate, package, and upload the .vsix as an artifact so a theme change can be installed and looked at before merging. release.yml runs on a v* tag: same checks, then publish to both registries and cut a GitHub release with the .vsix attached. Tagging is deliberate, and the tag must match package.json or the job fails. The drift gate covers only the five themes this repo's generator owns. Material and the four Flexoki variants come from tinte and are not reproducible from src/, so including them would fail every run and pressure someone into reverting tinte to make CI green. Both gates were verified by tampering with a generated file and observing exit 1, and by a clean run observing exit 0. Needs VSCE_PAT and OVSX_PAT as repository secrets.
Reworked from #71 by @helloAmulya, whose diagnosis and color choices this keeps verbatim. Two things blocked that PR from merging as it stood. It did not compile: the comment.cpp block was missing its closing braces, which tsc reports as TS1136 on line 852. The CI added in this branch would have caught it the day it was opened. It also carried +724/-320 in OneHunter-Material because running the build regenerates the five themes that tinte owns, which reverts them to their pre-tinte state. That is a property of the repo, not a mistake by the author. Restricting the change to the generator-owned themes makes it 126 purely additive lines across Classic, Light, and Vercel, with no deletions. Closes #70.
Without this, a missing secret surfaces as a 401 from vsce or ovsx partway through the job, after the other registry has already been published to, leaving the two stores on different versions. The check runs before packaging, so the job fails with both registries untouched and a message naming the missing secret.
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.
Why now
A fix merged in November 2024 never reached anyone. #68 by @peguimasid made local variables readable in Vercel 2024 Dark, and it has been sitting in
mainfor fifteen months while the published extension still ships the broken color.Verified by downloading the published
.vsixand comparing it againstHEAD:variable.other.local#171717, nearly black on a dark backgroundmain#EDEDEDPublishing has been manual, which is exactly how this happens.
What was broken
The generators reported success on failure. Writes inside
forEachwere never awaited and neither entry point had an error handler, so a failed write still exited0. Verified by injecting a failing write: it exited0before, and exits1now.The iTerm2 generator had never run.
xmlbuilderwas imported but never declared, so the step crashed every time and the crash was swallowed by the bug above. It runs now and reproduces the four committed.itermcolorsfiles byte for byte.vsceandovsxwere never declared, so the publish scripts only worked if they happened to be installed globally.The package was 7 MB.
.vscodeignoreexcluded almost nothing, so every publish carriedscreenshots/,src/,demos/, andnode_modules/.vscealso refused to package at all, becausedemos/.envtripped its secret check. Both fixed by the ignore list; it is 57 KB now, a 99% reduction. The README is unaffected since every image it references is a remote URL.C++ colors, reworked from #71
Credit to @helloAmulya for the diagnosis and the color choices, which this keeps verbatim. Two things blocked that PR:
comment.cppblock was missing its closing braces:TS1136on line 852. The CI in this PR would have caught it on day one.Scoped to the generator-owned themes, it is 126 purely additive lines across Classic, Light, and Vercel, zero deletions.
Closes #70.
CI, closing #2
Open since December 2022.
ci.ymlon every PR: typecheck, regenerate, drift gate, package, and upload the.vsixas an artifact so a theme change can be installed and looked at before merging.release.ymlon av*tag: same checks, publish to both registries, cut a GitHub release with the.vsixattached. The tag must matchpackage.jsonor the job fails.The drift gate deliberately covers only the five themes this repo's generator owns. Material and the four Flexoki variants come from tinte and are not reproducible from
src/, so gating them would fail every run and pressure someone into reverting tinte to make CI green.Verification
npm run generateend to end#EDEDEDBefore merging
release.ymlneeds two repository secrets, neither of which exists yet:VSCE_PAT, an Azure DevOps token, Marketplace scope Manage. The old one is likely expired since the last publish was fifteen months ago.OVSX_PAT, from open-vsx.org. TheRaillyHugonamespace is alive, so this only needs a new token, not a new namespace.Tag
v1.4.1once those are set.