Skip to content

Add chart versioning policy - #941

Draft
bschwedler wants to merge 2 commits into
mainfrom
version-policy
Draft

bschwedler wants to merge 2 commits into
mainfrom
version-policy

Conversation

@bschwedler

Copy link
Copy Markdown
Contributor
  • Enforce <= 1.0.0 in release
  • Add versioning policy

@lachlansimpson

Copy link
Copy Markdown

Thanks for picking this up, and for writing the convention down. The policy text
matches what you described on the issue, and routing it through
rstudio.description is a nice touch: one edit covers all six READMEs.

A few things before it leaves draft:

CONTRIBUTING.md / CLAUDE.md. The policy landed in consumer-facing docs,
but the person choosing minor vs. patch is reading CONTRIBUTING.md, which
today only says that a bump is required, not which position. Could the same
text go there? While it's open, two lines in it are stale: the bump rule no
longer covers docs-only changes (#839), and the file:// coupling was replaced
by pinned deps (#601).

The two non-product charts. "Product versions belong in appVersion" now
renders in the rstudio-library and rstudio-launcher-rbac READMEs, where
appVersion is SemVer and tracks no product. A sentence scoping that line to
the three product charts would cover it.

The pre-1.0.0 guard. I read it as an interlock, not a stance on graduation:
the release job hard-codes a patch bump, so failing early beats publishing a
wrong patch. If that's the intent, could the error message say so? As written it
fails a state the policy on the same PR describes as valid, which will confuse
whoever hits it.

The CI check (proposal 2). Is it planned as a follow-up?
check-news-entries.sh already has both versions and the NEWS entry in hand, so
the BREAKING/DEPRECATED match is a small addition. It's the piece that would
have caught 0.7.2, 0.20.1, 0.20.10, and workbench 0.21.1. Happy for it to be
separate, just want to know it isn't dropped.

Two tiny ones: the PR body says "<= 1.0.0" where the code enforces "< 1.0.0",
and the **/*.quarto_ipynb gitignore entry looks unrelated.

run: |
CHART_VERSION=$(yq -r '.version' "charts/${CHART_NAME}/Chart.yaml")
MAJOR=${CHART_VERSION%%.*}
if ! [[ "$MAJOR" =~ ^[0-9]+$ ]] || [ "$MAJOR" -ge 1 ]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Forever? or is this just a stop gap for now to prevent third parties from bumping us to 1.x against our will?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is just a stop-gap until we reach 1.0. I can add a comment or file an issue to remove this later.

@ianpittwood ianpittwood left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

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.

Write down and enforce the chart versioning convention

4 participants