Skip to content

docs: add Helm to CONTRIBUTING prerequisites for chart tests - #571

Merged
cevheri merged 3 commits into
libredb:mainfrom
SatvikMishra08:docs/contributing-helm-prerequisites
Sep 8, 2026
Merged

docs: add Helm to CONTRIBUTING prerequisites for chart tests#571
cevheri merged 3 commits into
libredb:mainfrom
SatvikMishra08:docs/contributing-helm-prerequisites

Conversation

@SatvikMishra08

Copy link
Copy Markdown
Contributor

Summary

CONTRIBUTING.md listed Bun/Node and Git, then told contributors to run bun run test. Chart unit tests need Helm 4.1.3 (CI pin) and helm dependency build for the gitignored PostgreSQL subchart. Without them, a fresh clone gets ~166 helm PATH failures (see #569).

Docs-only:

  • Add Helm 4.1.3 + helm repo add / helm dependency build … --skip-refresh under Prerequisites
  • Note the failure signature next to bun run test, and prefer bun run test:ci for a CI-matching check
  • Document the stale docker login → 401 trap

Closes #570

Test plan

  • Fresh clone follow Development Setup → Helm install + dependency build → chart unit files runnable
  • Skim CONTRIBUTING: Helm appears once in Prerequisites; bun run test note does not duplicate the Helm Chart Changes section

Docs-only change; local bun run test:ci not run (full suite is heavy for a CONTRIBUTING.md edit).

@cevheri cevheri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this matches what was measured on a fresh checkout: the twelve files, the 166 errors, the two commands and the 401 trap are all right. Two small changes before merge, both attached as suggestions you can apply with one click.

  1. The bun run test line in the scripts block is one long comment and carries two phrases that will not age well: the single-process failures are deterministic rather than a flake, and nothing tracks them yet. How the test script itself gets fixed is a maintainer decision and will be written down where it lands, so keep this line to the Helm fact and let the test:ci line say what it is.
  2. The file count in Prerequisites reads as thirteen (the glob plus two names); ten of the eleven helm-chart-* files spawn helm, so name the ten.

Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
@cevheri

cevheri commented Sep 6, 2026

Copy link
Copy Markdown
Member

main moved this afternoon: 0.13.7 became 0.14.0 and the chart 0.1.58 became 0.1.59, tagged at 14:05 +03. This branch is 22 commits behind and still carries Chart.yaml version: 0.1.58 / appVersion: "0.13.7".

Your checks are green, but they last ran on 2026-09-05, a day before that tag, so they describe a main that no longer exists.

Please take current main before the next round. From a fork:

git remote add upstream https://github.com/libredb/libredb-studio.git   # once
git fetch upstream
git merge upstream/main          # or: git rebase upstream/main
git push                         # after a rebase: git push --force-with-lease

Why now rather than at merge time: the chart version sync guard is the first step of Lint, Typecheck and Build, and CI fetches main with --depth=1, so it compares your Chart.yaml against main's tip rather than against your merge base. libredb-studio-0.1.58 is a released tag, so the guard refuses it. When that step fails the job stops there and the eleven gates behind it (the three other drift guards, format, lint, typecheck, knip, build, build:lib, attw and the Go launcher tests) come back unmeasured rather than green. Measured on #582 earlier today; merging main cleared it.

@cevheri

cevheri commented Sep 7, 2026

Copy link
Copy Markdown
Member

any update

@cevheri

cevheri commented Sep 8, 2026

Copy link
Copy Markdown
Member

Updated this branch myself rather than leaving it to you: it was 85 commits behind and the review had been open two days. Your commit is untouched.

Both script-block suggestions went in verbatim. The Prerequisites line I wrote from the review body, not from my own suggestion text, because the two disagreed and the suggestion was wrong.

Measured with helm hidden from PATH, per file:

  • 166 is right, but only in the state the paragraph describes: fresh clone, no helm dependency build. With the subchart already built it is 169.
  • It is ten files, not twelve, and they are all helm-chart-*.test.ts.
    distribution-check.test.ts and generate-channel-showcase.test.ts never spawn the binary; "helm" there is a channel id in a YAML fixture. So is
    helm-chart-readme-recipes.test.ts, which is a static lint. The other ten sum to exactly 166.

All eight local gates clean. Merging once checks are green.

@cevheri

cevheri commented Sep 8, 2026

Copy link
Copy Markdown
Member

@SatvikMishra08 Worth saying: your 166 and your 401 trap both held up under measurement, and that is the hard part of a docs fix. Keep contributing, it sharpens exactly this.

@cevheri
cevheri merged commit db40452 into libredb:main Sep 8, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CONTRIBUTING prerequisites omit Helm, which 166 tests in bun run test need

2 participants