ci: align the 3.x version-check grammar with 4.x (DRIVER-1070) - #1066
Draft
nikagra wants to merge 1 commit into
Draft
ci: align the 3.x version-check grammar with 4.x (DRIVER-1070)#1066nikagra wants to merge 1 commit into
nikagra wants to merge 1 commit into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟡 Changes recommended
Validation must account for CCM’s global architecture-token removal.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Aligns the 3.x server-version validation grammar with 4.x.
Changes:
- Accepts dated build identifiers.
- Tightens prerelease and separator validation.
- Updates validation error messages.
File summaries
| File | Review |
|---|---|
Makefile |
Updates version validation, but permits architecture tokens that CCM can normalize into moving selectors. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # grammar decides all of them. | ||
| SERVER_VERSION_RE = ^[0-9]+\.[0-9]+\.[0-9]+$$|^[0-9]+\.[0-9]+(\.[0-9]+)?[-~.][A-Za-z][A-Za-z._~-]*[0-9][A-Za-z0-9._~-]*$$ | ||
| _VERSION_NUM = [0-9]+\.[0-9]+(\.[0-9]+)? | ||
| _VERSION_LABEL = [A-Za-z]+[0-9][A-Za-z0-9]* |
scylladb#1044 merged the shared grammar on 3.x, then review of the 4.x twin scylladb#1049 tightened it, leaving the two release lines checking different things. A pre-release label must carry its own number: CCM strips a trailing -x86_64, so 6.2.0-dev-aarch64 would reach it as the moving 6.2.0-dev. A bare label is exact only before a dated build id such as 2024.2.3-0.20250108.931ce203dcf5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vGi2zZJxw51iagdPmVa4t
nikagra
force-pushed
the
fix/DRIVER-1070-align-version-grammar
branch
from
September 9, 2026 11:23
3f838df to
5182e3a
Compare
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.
#1044 landed the shared version grammar on 3.x. Review of the 4.x twin #1049 then tightened that same grammar after #1044 had merged, and #1049 has now merged, so 3.x is the lagging line — and the drift is not cosmetic. Scored against the tightened grammar, the pattern on
scylla-3.xtoday gets 60 of 77 cases wrong in both directions.-x86_64/-aarch64before parsing (ccmlib/utils/version.py), so6.2.0-dev-aarch64passes the current 3.x check and then reaches CCM as the moving selector6.2.0-dev.2024.2.3-0.20250108.931ce203dcf5are now accepted; the current pattern rejects them outright even though CCM recognises the form, so a real dated build fails the build.5.4.0~dev-0.20230801.37b548f46365keeps working.6.2.0-rc.1) are rejected, and a trailing suffix may no longer end in a separator (6.2.0-rc0-,6.2.0-rc0.x86_64.).Verified:
SERVER_VERSION_REexpands byte-identically on both lines, and the grammar block and all three messages are byte-identical to merged 4.x. 77-case corpus plus tail-boundary probes; every accepted string put through ccmlib itself — of 41, 39 parse to an exact version, 2 fail loudly, none becomes a moving selector; each intended case shown to flip against the pre-patch file;resolve-*-versionanddownload-scyllaexercised end to end against a stubbedget-version(10 cases). No integration suite runs locally, so the realccm createpath rests on CI. CI is unaffected either way — 3.x lanes pass only aliases.Twin of #1049 (4.x), merged as f2890a9, which carries the identical grammar.
Refs: #1049