Skip to content

#278 add upper version bounds for fbe and judges - #278

Open
VasilevNStas wants to merge 1 commit into
zerocracy:masterfrom
VasilevNStas:278-gemfile-constraints
Open

VasilevNStas wants to merge 1 commit into
zerocracy:masterfrom
VasilevNStas:278-gemfile-constraints

Conversation

@VasilevNStas

Copy link
Copy Markdown
Contributor

Both fbe and judges are pre-1.0 gems where minor version bumps may introduce breaking changes — API changes, CLI option changes, S-expression query syntax changes, and exit code changes.

The problem

# Before: no upper bound — could jump to 1.0 or higher
gem 'fbe', '>0'
gem 'judges', '>0'

If someone runs bundle update (accidentally or during Docker build without a lock file), versions could jump to a hypothetical 1.0 with breaking changes.

The fix

# After: constrained to 0.x series
gem 'fbe', '~>0'
gem 'judges', '~>0'

~>0 means >= 0 and < 1 — allows all 0.x releases but prevents jumping to 1.0.

Impact

Operation Before After
bundle install (with lock) Exact version from lock Same — lock wins
bundle update Could jump to 1.0 Stays within 0.x
bundle install (without lock) Latest version Latest 0.x

The Gemfile.lock already pins exact versions, so production builds are unaffected. This change protects the next bundle update from surprise breakage.

Checklist

  • bundle exec rubocop — 0 offences
  • bundle exec rake — all tasks pass
  • HoC ≤ 133

@yegor256 please review

@VasilevNStas
VasilevNStas requested a review from yegor256 as a code owner June 29, 2026 17:00
VasilevNStas added a commit to VasilevNStas/swarm-template that referenced this pull request Jun 29, 2026
Both fbe and judges are pre-1.0 gems where minor version bumps may
introduce breaking changes (API changes, CLI option changes, S-expression
query changes, exit code changes).

Changing from `>0` to `~>0` constrains updates to the 0.x series,
preventing `bundle update` from accidentally jumping to a hypothetical
1.0 release. The Gemfile.lock already pins exact versions, but the
Gemfile constraint protects against lockfile regeneration.

Other gems with `>0` (rubocop-*) are dev-only tools and less critical.
@VasilevNStas
VasilevNStas force-pushed the 278-gemfile-constraints branch from 4939756 to efcb12b Compare June 29, 2026 17:06
VasilevNStas added a commit to VasilevNStas/swarm-template that referenced this pull request Jun 29, 2026
Both fbe and judges are pre-1.0 gems where minor version bumps may
introduce breaking changes (API changes, CLI option changes, S-expression
query changes, exit code changes).

Changing from `>0` to `~>0` constrains updates to the 0.x series,
preventing `bundle update` from accidentally jumping to a hypothetical
1.0 release. The Gemfile.lock already pins exact versions, but the
Gemfile constraint protects against lockfile regeneration.

Other gems with `>0` (rubocop-*) are dev-only tools and less critical.
@VasilevNStas
VasilevNStas force-pushed the 278-gemfile-constraints branch from efcb12b to ae0e517 Compare June 29, 2026 17:09
Both fbe and judges are pre-1.0 gems where minor version bumps may
introduce breaking changes (API changes, CLI option changes, S-expression
query changes, exit code changes).

Changing from `>0` to `~>0` constrains updates to the 0.x series,
preventing `bundle update` from accidentally jumping to a hypothetical
1.0 release. The Gemfile.lock already pins exact versions, but the
Gemfile constraint protects against lockfile regeneration.

Other gems with `>0` (rubocop-*) are dev-only tools and less critical.
@VasilevNStas
VasilevNStas force-pushed the 278-gemfile-constraints branch from ae0e517 to 97b7d67 Compare June 29, 2026 17:11
@VasilevNStas

Copy link
Copy Markdown
Contributor Author

@yegor256 — both fbe and judges are pre-1.0 gems where minor bumps may introduce breaking changes (CLI options, exit codes, S-expression syntax). The Gemfile had >0 with no upper bound. ~>0 constrains to the 0.x series, preventing bundle update from jumping to a hypothetical 1.0. Bonus: updates fbe 0.47→0.48 and judges 0.58→0.60 in the lockfile.

@Favixx Favixx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed at 97b7d67.

The constraint does not do what the description says it does. ~>0 expands to >= 0, < 1, so it excludes only a 1.0 release of fbe/judges that does not exist and has no pre-release on rubygems; current releases are fbe 0.49.1 (2026-07-17) and judges 0.61.1 (2026-07-24), both admitted by both the old and the new constraint. Meanwhile the breakage the description warns about -- "pre-1.0 gems where minor version bumps may introduce breaking changes" -- happens inside 0.x, and this very diff contains two examples: fbe 0.47.0 -> 0.48.5 moved its own factbase requirement from ~> 0.11 to ~> 0.19, and judges 0.58.1 -> 0.60.4 added octokit ~> 10.0 and ellipsized ~> 0.3 as new runtime dependencies. ~>0 permits both. If the premise is right, the bound should be ~> 0.48.5 / ~> 0.60.4 (three segments -- ~> 0.48 is still < 1.0). If a per-minor pin is too tight for a template people are meant to keep current, then the lockfile is the right mechanism and these two lines should stay >0.

The lockfile diff is not the diff this change requires. Switching >0 to ~>0 cannot alter resolution, so the required lock change is the two DEPENDENCIES lines. What landed is a full bundle update: roughly thirty gems move, including three majors that nothing in the tree asks for -- bigdecimal 3.3.1 -> 4.1.2, connection_pool 2.5.5 -> 3.0.2, public_suffix 6.0.2 -> 7.0.5. Renovate's own fresh resolution in open PR #288 keeps bigdecimal at 3.3.1, which confirms these are side effects of an unconstrained relock. A stray x86_64-darwin-24 platform plus darwin builds of ffi, nokogiri and sqlite3 came from the author's machine; every workflow is ubuntu-24.04 and entry.sh is a Linux container entry point. The description's claim that "the Gemfile.lock already pins exact versions, so production builds are unaffected" is true of the constraint and false of this pull request, which rewrites those pins wholesale.

Scope. The README.md hunk is byte-for-byte identical to open PR #290 by the same author ("fix markdownlint violations in README.md"), so whichever merges second conflicts and the same eight lines get reviewed twice. The fbe/judges version moves duplicate open renovate PRs #288 and #289, both of which are Gemfile.lock-only and scoped to one gem each.

On the underlying goal. If the aim is that an accidental bundle update cannot silently change what runs, the missing piece is frozen mode, not a range. Nothing in the repo sets bundle config set frozen true or BUNDLE_FROZEN, and .github/workflows/rake.yml runs ruby/setup-ruby with bundler-cache: true followed by a plain bundle install --no-color -- neither is frozen, so Gemfile/lock drift re-resolves silently and CI stays green. That change is smaller, it is actually enforced by CI, and it covers all eleven gems rather than two. It also deserves to be settled as policy rather than per-gem, because swarm-template is a real GitHub template repo (is_template: true, "Use this repository as a template to create your own swarm of judges") and whatever lands here is copied into every generated swarm and expensive to relax afterwards. As it stands the policy is half-applied: rubocop-minitest, rubocop-performance and rubocop-rake are left at >0 on lines 14-16 even though rubocop-minitest is pre-1.0 too (renovate #294 raises it to 0.40.0).

15 inline comments below.

Comment thread Gemfile
@@ -5,8 +5,8 @@

source 'https://rubygems.org'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Worth being explicit about what this file is before deciding the policy question. swarm-template is a GitHub template repository (is_template: true; the description reads "Use this repository as a template to create your own swarm of judges"), so every gem line below this one is copied verbatim into each generated swarm and then diverges independently. A constraint added here is not a decision for one repo, it is a default for every swarm created from this point forward, and those copies get no notification when the constraint becomes wrong. Relaxing it later means a PR in each downstream swarm individually. That asymmetry argues for the template shipping the loosest constraint that is still honest, and for reproducibility to be enforced by the lockfile rather than by a range.

Which is the more useful point: the reproducibility problem the description describes is already solved by Gemfile.lock, and the one real gap in that solution is not addressed by this PR. Nothing in the repo enables frozen mode -- there is no bundle config set frozen true, no --deployment, and .github/workflows/rake.yml uses ruby/setup-ruby with bundler-cache: true followed by a plain bundle install --no-color. In that setup, if Gemfile and Gemfile.lock disagree, Bundler silently re-resolves and rewrites the lock in the CI workspace instead of failing. A violated constraint is therefore not something CI can catch today, and the constraint being added here is not something CI will ever exercise.

If the goal really is "an accidental bundle update must not silently change what runs in production", the change that delivers it is BUNDLE_FROZEN: 'true' in the rake job env (or bundle config set --local frozen true committed to .bundle/config), so that any Gemfile/lock drift fails the build loudly. That is a smaller diff than this one and it covers all eleven gems instead of two.

Comment thread Gemfile

gem 'fbe', '>0'
gem 'judges', '>0', require: false
gem 'fbe', '~>0'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

~>0 expands to >= 0, < 1, so the only thing this excludes is a 1.0 release of fbe. fbe is at 0.49.1 (released 2026-07-17) and has been in the 0.x series for its entire life; there is no 1.0 on rubygems and no pre-release of one. As written this constraint is a no-op against every version that exists, and it stays a no-op right up until the day it isn't -- at which point it blocks an upgrade rather than making one safe.

The description gets the risk analysis right and then applies the wrong operator to it: "pre-1.0 gems where minor version bumps may introduce breaking changes". If that premise holds -- and it does; see line 77 of the lockfile in this same diff, where fbe moved its factbase requirement from ~> 0.11 to ~> 0.19 across 0.47.0 -> 0.48.5 -- then the transitions that need bounding are 0.48 -> 0.49, not 0.99 -> 1.0. ~>0 permits exactly the transitions the description warns about and blocks only the one it never discusses.

Expressing the stated intent in Ruby needs three segments: ~> 0.48.5 means >= 0.48.5, < 0.49, the pre-1.0 analogue of a caret bound. Note that ~> 0.48 would not do it -- that is >= 0.48, < 1.0, the same practical effect as what is written here. So the choice is a genuine one between ~> 0.48.5 (real gate, needs deliberate raising every minor) and leaving >0 and trusting the lockfile. ~>0 is the option that has the maintenance cost of a constraint without the protection of one.

Comment thread Gemfile
gem 'fbe', '>0'
gem 'judges', '>0', require: false
gem 'fbe', '~>0'
gem 'judges', '~>0', require: false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The case for bounding judges is stronger than for fbe, because judges is consumed as a CLI rather than as a library. entry.sh invokes judges update --summary --max-cycles=3 --no-log --option "id=${id}" --lib ... and the Rakefile invokes judges test --no-log --disable live --lib lib judges. A renamed or removed flag breaks the container at runtime, not at bundle install, and there is no test in this repo that would catch it (rake judges uses the same flags, so it fails together with production rather than ahead of it). That is precisely the failure mode the description names -- and ~>0 does not prevent it, because such a rename ships as 0.60 -> 0.61.

This diff carries the evidence. judges moves 0.58.1 -> 0.60.4 in the lockfile, and that minor range added two new runtime dependencies: ellipsized ~> 0.3 and octokit ~> 10.0 (lock lines 120 and 127). Current released judges is 0.61.1 (2026-07-24), which ~>0 also admits silently.

If you want a bound that actually works for judges, ~> 0.60.4 is the one that matches the argument in the description, and it should come with a line in README.md telling downstream swarm authors to raise it deliberately after reading the judges changelog. Leaving it at ~>0 means the flag-rename scenario is still wide open while the repo now carries a constraint that reads as protection.

(require: false is correct and unaffected -- judges is only ever shelled out to, never required.)

Comment thread Gemfile
gem 'judges', '>0', require: false
gem 'fbe', '~>0'
gem 'judges', '~>0', require: false
gem 'minitest', '~>6.0', require: false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Every other constrained gem in this file uses a two-segment pessimistic bound: ~>6.0 on this line, then ~>1.7, ~>13.2, ~>1.75, ~>0.22, ~>3.0. ~>0 is the only single-segment constraint in the file, and single-segment ~> is rare enough in practice that most readers will parse it as "pinned to 0.x somehow" rather than as the precise >= 0, < 1 it is. If a bound stays at all, writing it as ~>0.48 / ~>0.60 costs nothing and is self-documenting -- a reader can see which release the constraint was reasoned about against, and git blame on the line gains meaning.

There is also an internal inconsistency this PR leaves in place. Three gems on lines 14-16 are still >0: rubocop-minitest, rubocop-performance, rubocop-rake. rubocop-minitest is pre-1.0 -- open renovate PR #294 raises it to 0.40.0 -- so the "pre-1.0 gems may break on minor bumps" argument applies to it verbatim, and it is a gem that gates the build via rake rubocop. Either the argument justifies bounding all pre-1.0 dependencies or it justifies bounding none; picking two and leaving a third makes the policy unreadable to whoever edits this file next.

This is worth resolving explicitly rather than by omission, because in a template repo this file is the policy statement that every generated swarm inherits.

Comment thread Gemfile
gem 'judges', '~>0', require: false
gem 'minitest', '~>6.0', require: false
gem 'minitest-reporters', '~>1.7', require: false
gem 'rake', '~>13.2', require: false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This changes how renovate treats these two lines, and the repo already contains a controlled experiment for both behaviours. renovate.json is a bare config:recommended with no rangeStrategy set, so the bundler manager uses its default: a gem whose constraint already admits the new version gets a Gemfile.lock-only PR, and a gem whose constraint excludes it gets a PR that rewrites the Gemfile line too. Compare open PR #294 (rubocop-minitest to 0.40.0, constraint >0) which touches Gemfile.lock only, against #291 (simplecov to v1, constraint ~>0.22) and #292 (simplecov-cobertura to v4, constraint ~>3.0), both of which rewrite Gemfile.

So the effect of ~>0 is deferred rather than absent. While fbe and judges stay in 0.x, renovate keeps sending lockfile-only PRs exactly as it does today (#288 for fbe 0.49.1, #289 for judges 0.61.1, both Gemfile.lock-only) -- no new churn. On the day either gem cuts 1.0, renovate opens a PR that changes ~>0 to ~>1.0 and relocks, and a maintainer merges it the same way #291 and #292 will be merged: without the changelog review the bound existed to force.

That is the honest cost/benefit -- the bound buys one automated pull request's worth of friction, priced against a constraint that must be maintained here and in every generated swarm. If a real review gate is what is wanted, put it in renovate.json where it is legible: "rangeStrategy": "update-lockfile" scoped to these two packages, or "dependencyDashboardApproval": true for them, both of which state the intent instead of encoding it in a version range that a bot will rewrite.

Comment thread Gemfile.lock
yaml (0.4.0)

PLATFORMS
x86_64-darwin-24

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

x86_64-darwin-24 was added to PLATFORMS, bringing with it darwin builds of three native gems: ffi (line 96), nokogiri (line 155) and sqlite3 (line 216). This is an artifact of the lock being regenerated on a Mac, not a decision -- nothing in the repo targets macOS. Every workflow in .github/workflows/ runs ubuntu-24.04, .rultor.yml builds inside yegor256/rultor-image:1.24.0, and entry.sh is the entry point for a Linux container.

It also does not achieve the thing it superficially looks like it achieves. x86_64-darwin-24 covers Intel Macs only; an Apple Silicon contributor is on arm64-darwin-24 and still hits a platform mismatch on bundle install. So the lock now carries three extra native gem entries that help no one on the team and no one downstream, and the next contributor who runs bundle lock on Linux either strips them or leaves them -- churning this file either way, in a template repo where this file is copied into every new swarm.

bundle lock --remove-platform x86_64-darwin-24 before pushing. If cross-platform local development is genuinely wanted in the template, that is a separate PR that adds both darwin architectures deliberately and documents the intent in README.md, rather than one architecture arriving as a side effect of where the author happened to run bundler.

Comment thread Gemfile.lock
DEPENDENCIES
fbe (> 0)
judges (> 0)
fbe (~> 0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These two lines are the entire lockfile change this PR actually requires -- everything from line 4 to line 223 is incidental to it. That is worth stating in the description, because as things stand a reviewer has to reason about the diff to work out which parts follow from the Gemfile edit and which parts are an unrelated bundle update.

This section is also where the limit of the whole approach shows. Bundler records the constraint here, but nothing verifies that the record still matches Gemfile. rake.yml runs ruby/setup-ruby with bundler-cache: true and then bundle install --no-color; neither is frozen, so if someone later edits a constraint and forgets to relock, Bundler re-resolves in the workspace, the build stays green, and the committed DEPENDENCIES block silently goes stale. The bound this PR adds is enforced by nothing except a human reading git diff.

Note also that renovate PRs #291 and #292 will rewrite the simplecov (~> 0.22) and simplecov-cobertura (~> 3.0) lines a few rows below this one, together with their Gemfile counterparts -- so this section is already a small conflict surface, and a 42-line lock diff on top of it makes rebasing this PR more expensive than the two-line change it should be.

Comment thread README.md
## Project Structure

```
```text

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This whole README hunk -- the text language on this fence, the four 1. renumberings, the table delimiter row, and the MD013 disable pair -- is byte-for-byte identical to open PR #290 by the same author ("#284 fix markdownlint violations in README.md"), which touches README.md and nothing else. Two open PRs carrying the same diff means whichever merges second conflicts, and the same eight lines get reviewed twice by whoever ends up reviewing both.

Drop README.md from this PR and let #290 land it. That also removes the only reason markdown-lint is green here: the check is currently failing on master (10 errors), and bundling the fix into a dependency PR means the red build gets repaired as a side effect of an unrelated change, under the wrong issue number, with the repair invisible in the PR title. It should be fixed by the PR that exists for exactly that purpose.

The fence change itself is correct -- markdownlint reports MD040/fenced-code-language on this line against master, and text is the right choice for a directory listing rather than inventing a language.

Comment thread README.md
```

2. Write the judge logic in `judges/hello-world/hello-world.rb`:
1. Write the judge logic in `judges/hello-world/hello-world.rb`:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is the markdownlint-correct fix and simultaneously a rendering regression. MD029/ol-prefix fires on master at lines 44, 59, 77 and 83 because each fenced code block sits at column 0, which terminates the list -- so 2., 3., 4. and 5. are each the first item of a brand-new ordered list, and markdownlint requires a list to start at 1. Renumbering them all to 1. silences the rule.

But GitHub honours the start number of an ordered list. 2. Write the judge logic currently renders as step 2; 1. Write the judge logic renders as step 1. After this change the "Adding a New Judge" section shows five consecutive steps all labelled "1.", which is a worse document than the one with the lint warnings -- and this is the section that teaches every downstream swarm author how to write their first judge.

The fix that satisfies both is to make it a single list again by indenting each fenced block three spaces so it sits inside its preceding list item. Then the numbering can stay 1. through 5.: MD029's default one_or_ordered style accepts a properly incrementing list, and the rendered page reads as five steps instead of five step-ones. The same change is needed at lines 59, 77 and 83.

Comment thread README.md

## Commands

<!-- markdownlint-disable MD013 -->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The blanket disable is considerably wider than the problem. Running markdownlint against master reports exactly one MD013/line-length violation in this file -- the bundle exec judges test --no-log --disable live --lib lib judges row, at 89 characters against the default limit of 80. This pair suppresses the rule across all five table rows, including the four that are comfortably under, so a future row that runs to 200 characters lands silently.

In a template repository this is also the wrong place for the setting. There is no .markdownlint.json or .markdownlint-cli2.jsonc in the repo, so DavidAnson/markdownlint-cli2-action runs on pure defaults. Adding a config with MD013: { "tables": false } (or a raised line_length) fixes the class of problem once, applies to every markdown file including the ones downstream swarm authors add, and is inherited cleanly. The inline disable/enable pair instead gets copy-pasted into every generated swarm's README, where nobody will remember what it was for.

The delimiter reformat on line 95 is a genuine fix and should stay wherever this hunk lands -- MD060/table-column-style fires four times on master for the missing spaces around those pipes.

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.

2 participants