ci: run RuboCop and YARD once instead of on every matrix job - #1672
Merged
Conversation
RuboCop and YARD analyze source text, so their results are identical on every runtime: RuboCop's verdict is fixed by TargetRubyVersion, not the host Ruby. Repeating them across all six matrix jobs was the largest cost in the build. Measured from run 31204960715, RuboCop alone took 205s of the 403s JRuby job and 194s of the 347s TruffleRuby job. Moving it and YARD into a single ubuntu/Ruby 3.4 job cuts total matrix time from about 1313s to about 690s and the critical path from 403s to about 245s, now bounded by the Windows integration suite. Also: - Add concurrency groups so a new push cancels the in-progress run for the same PR, rather than leaving a full matrix running against a dead commit. - Drop Ruby 3.4 from the spec matrix. It is bracketed by the 3.2 and 4.0 runs, and it remains in CI as the host of the new lint job. - Host the lint job on Ruby 3.4, not 4.0. yard-lint needs 3.3 or later, and a merge gate is better insulated from churn in a brand-new major. - Correct the TargetRubyVersion comment, which said 3.1 while the value was 3.2 and described guarding against the shared config being raised when that config actually sets a lower value. Unit specs stay on every runtime. They cost 4-27s per job, and they are where the parsers get exercised against fixture output, which is exactly where the regexp and encoding behavior of MRI, JRuby, and TruffleRuby diverges. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR reduces duplicated CI work by moving linting and documentation checks out of the runtime matrix and into a single dedicated job, while also adding workflow-level concurrency cancellation to avoid running full matrices for outdated commits.
Changes:
- Add a dedicated “Lint and Docs” job (Ubuntu, Ruby 3.4) to run
rake rubocop yardonce per CI run, and switch matrix jobs to runrake spec. - Add workflow-level concurrency to cancel in-progress runs when a PR receives new pushes.
- Update
.rubocop.ymldocumentation aroundTargetRubyVersionto correctly explain the explicit pin and its relationship to the gem’s Ruby floor.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.rubocop.yml |
Clarifies why TargetRubyVersion is explicitly pinned to the gem’s minimum supported Ruby. |
.github/workflows/enforce_conventional_commits.yml |
Adds concurrency cancellation so only the latest PR state is linted. |
.github/workflows/continuous_integration.yml |
Introduces a single “Lint and Docs” job and trims the matrix to run specs only (dropping Ruby 3.4 from the spec matrix). |
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.
Motivation
Reduce the workload of PR CI runs. Profiling an actual run rather than guessing showed the cost was not in what the matrix tests, but in five jobs redundantly re-running the same lint.
From run 31204960715:
spec:unitspec:integrationrubocopRuboCop was 51% of the JRuby job and 56% of the TruffleRuby job. It is pure duplicated work: RuboCop analyzes source text, and its verdict is fixed by
TargetRubyVersion(pinned to 3.2), not by the host Ruby. The same is true of YARD.Changes
Lint and Docsjob on ubuntu / Ruby 3.4 runsrake rubocop yardonce. Matrix jobs runrake specinstead ofrake default.TargetRubyVersioncomment corrected. It said 3.1 while the value was 3.2, and described guarding againstmain_branch_shared_rubocop_configbeing raised, when that config actually sets a lower value (3.1). The explicit pin is still required, because a value inherited viainherit_gemtakes precedence over inference from the gemspec.Why Ruby 3.4 hosts the lint job
yard-lintis only installed on Ruby 3.3+ (per the gemspec), and YARD cannot build on JRuby or TruffleRuby, so the host must be 3.3+. Choosing 3.4 over 4.0 keeps a merge gate insulated from churn in a brand-new major; Ruby 4.0 deprecations still surface through the matrix, which runs 4.0.Side benefit:
yard:lintnow always runs. The old 3.2 job silently skipped it.Measured result
Both figures below are warm-bundler-cache runs, so they are directly comparable: run 31204960715 had cache hits on all six jobs, and the numbers below come from attempt 2 of the run on this branch, which had cache hits too.
Per job, after: Lint and Docs 62s, 3.2 ubuntu 35s, 4.0 ubuntu 33s, truffleruby 163s, jruby 200s, 3.2 windows 238s.
The critical path is now Windows, bounded by its integration suite -- inherent process-spawn cost, not waste.
What was deliberately not done
release-please--guards would need rewriting first, sincemerge_groupevents carry nogithub.event.pull_request.head.ref.Verification
Locally:
rake rubocop yardpasses (RuboCop clean, plusyard:build,yard:lint,yard:example-test);rake specpasses with 5,816 unit and 569 integration examples at 100% line and branch coverage. Coverage of the oldrake defaultis complete across the two job types, withbuildstill running on every platform as a prerequisite oftest:gem.On this PR, confirm that the
Lint and Docslog contains arake yard:lintbox and that no matrix job log contains arake rubocopbox.Note on required checks
Unrelated to this diff, but surfaced while verifying it: the
Release Branchruleset requires onlyVerify Conventional Commits, so CI is not currently a merge gate and a PR with a red matrix is mergeable. That job's name is unchanged here, so nothing breaks. If CI should gate merges, the new names to add areLint and Docs,Ruby 3.2 on ubuntu-latest,Ruby 4.0 on ubuntu-latest,Ruby truffleruby-24.2.1 on ubuntu-latest,Ruby jruby-10.0.0.1 on ubuntu-latest, andRuby 3.2 on windows-latest.Follow-up found while verifying (not addressed here)
This workflow triggers only on
pull_request, sobundler-cachewrites land underrefs/pull/<n>/mergeand are never shared between PRs. Only PRs whose base branch has a cache at default-branch scope get a hit -- currently just Ruby 4.0, populated incidentally byrelease.yml(ruby-version: ruby, which resolves to 4.0.6) during the v5.0.4 release.The first run on this branch showed the cost: five of six jobs cold-installed gems,
Setup Rubytook 293s of 925s total, and Windows alone spent 109s there versus 9s for the cache-hitting 4.0 job. Populating caches at default-branch scope looks worth roughly 240s of total time and 100s off the critical path -- comparable to this PR. Left for a separate change.🤖 Generated with Claude Code