Modernize GitHub Actions setup, pin actions by SHA, and add a RuboCop baseline - #553
Open
tas50 wants to merge 2 commits into
Open
Modernize GitHub Actions setup, pin actions by SHA, and add a RuboCop baseline#553tas50 wants to merge 2 commits into
tas50 wants to merge 2 commits into
Conversation
Pin every action and reusable workflow to a full commit SHA with the
version in a trailing comment, so a moved tag cannot silently change
what runs in CI. Dependabot understands this format and keeps both the
SHA and the comment current.
actions/checkout v7 -> 3d3c42e5 (v7.0.1)
ruby/setup-ruby v1 -> 95ef2b04 (v1.321.0)
fog/.github ci.yml v1.5.0 -> f71b6243 (v1.6.0)
Note ruby/setup-ruby's v1 is a branch rather than a tag, so it was
previously not pinned to anything immutable at all.
Other changes to linting.yml:
- Add a permissions block. It had none, so the job ran with the
repository default token scope; ci.yml already restricts this.
- Run on pull_request as well as push. With on: [push] a pull request
from a fork was never linted.
- Run 'bundle exec rubocop' rather than bare 'rubocop', so the linter
is the version the bundle resolves.
- Enable bundler-cache for gem caching between runs.
- Move off Ruby 3.0, which is end-of-life, and quote the version so
YAML cannot read it as a float.
- Drop the stale third-party-actions boilerplate comment.
Both workflows also gain a concurrency group so superseded pull request
runs are cancelled, and workflow_dispatch so they can be triggered
manually.
Signed-off-by: Tim Smith <tsmith84@proton.me>
Linting has been failing on master for a long time: 2492 offenses across
1368 files, none of them error severity. Now that linting.yml also runs on
pull requests, that pre-existing failure would show up on every PR.
Generate a baseline so only *new* offenses fail:
bundle exec rubocop --auto-gen-config --no-exclude-limit --no-auto-gen-timestamp
Two flags are worth explaining.
--no-exclude-limit: at RuboCop's default limit of 15, any cop offending in
more than 15 files is written as `Enabled: false`, which switches the cop
off for the whole repository. That would have silently disabled 23 cops,
including Lint/UselessAssignment, Lint/AssignmentInCondition and
Metrics/AbcSize, so new code would never be checked for them either. With
no limit every cop gets an explicit file list instead, which grandfathers
the existing offenders while keeping the cop enforced everywhere else. It
also means RuboCop emits Exclude lists rather than raising Max, so the
metrics cops keep their configured thresholds instead of being reset to
the worst existing offender.
--no-auto-gen-timestamp: keeps a regeneration from producing a diff that
is nothing but a changed date.
The baseline is almost entirely style debt. Four entries are not, and are
tracked separately in fog#555 rather than being fixed here: a broken remote
template fetch in the Heat file loader, a mock/real signature mismatch in
Image V2 upload_image, duplicate attr_writer declarations in
Compute::Server, and a YAML.load in the introspection mock.
No source files are autocorrected; 1601 of the offenses are
autocorrectable, but that belongs in its own change.
Signed-off-by: Tim Smith <tsmith84@proton.me>
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.
Pins every action and reusable workflow to a full commit SHA with the version in a trailing comment, and modernizes
linting.yml.SHA pinning
A tag is a mutable pointer —
v7can be moved to different code at any time, so a version pin is a trust decision renewed on every run. A SHA cannot move. Dependabot understands the@<sha> # <version>format and keeps both parts current, so this costs nothing in maintenance.actions/checkout@v7@3d3c42e5aac5ba805825da76410c181273ba90b1# v7.0.1ruby/setup-ruby@v1@95ef2b042f9d7a56d8268cba8559e2842e2ad01b# v1.321.0fog/.githubci.yml@v1.5.0@f71b6243d92cf84c2307959de3016e0fb7b3411c# v1.6.0Worth calling out that the
v1ofruby/setup-rubyis a branch, not a tag — so it was not pinned to anything immutable at all.The
fog/.githubbump to v1.6.0 makes #549 redundant; happy to rebase onto that instead if you would rather land the Dependabot PR first.linting.yml fixes
permissions:block. The job ran with the repository default token scope.ci.ymlalready restricts this; now both do.on: [push]meant fork pull requests were never linted. Now runs onpull_requesttoo.rubocopinstead ofbundle exec rubocop, so the linter was not necessarily the version the bundle resolves.bundler-cache: truefor gem caching between runs.3.0is a YAML float, harmless at3.0but a trap at3.10.Both workflows also gain a
concurrencygroup so superseded PR runs are cancelled, andworkflow_dispatchso they can be triggered manually.Two pre-existing problems this PR does not fix
1. The CI workflow is disabled. The API reports:
It has zero runs, which is why the Actions badge renders
CI - no statusand why recent pull requests show no checks at all. GitHub disables scheduled workflows after a period of repository inactivity, and this cannot be re-enabled by a pull request — it needs a maintainer to press "Enable workflow" in the Actions tab. Theworkflow_dispatchtrigger added here gives a way to kick it manually once it is re-enabled.2. Linting is red and has been for a long time. Every recent run of
linting.ymlon master has failed. Locallybundle exec rubocopreports 2492 offenses across 1368 files:No offense is error-severity, so this is accumulated style debt rather than breakage. Because this PR makes linting run on pull requests, that pre-existing failure will now be visible on every PR rather than only on pushes to master.
This is now fixed here with a
.rubocop_todo.ymlbaseline, so only new offenses fail:--no-exclude-limitis the flag that matters. At RuboCop's default limit of 15, any cop offending in more than 15 files is written asEnabled: false— switching that cop off for the entire repository. That would have silently disabled 23 cops, includingLint/UselessAssignment,Lint/AssignmentInConditionandMetrics/AbcSize, so new code would never be checked for them either. With no limit, every cop gets an explicit file list: existing offenders are grandfathered, the cop stays enforced everywhere else. It also means RuboCop emitsExcludelists rather than raisingMax, so the metrics cops keep their configured thresholds instead of being reset to the worst existing offender.--no-auto-gen-timestampkeeps a future regeneration from producing a diff that is nothing but a changed date.No source files are autocorrected. 1601 of the 2492 offenses are autocorrectable, but that is a large mechanical diff that belongs in its own change.
Result:
1368 files inspected, no offenses detected.Four entries in the baseline are not style debt
Most of the baseline is hash alignment and line length. Four entries are latent defects that the baseline would otherwise bury, so they are tracked in #555 rather than fixed here:
Kernel#openlost URI support, soread_urivalidates a URL then hands it to a file-only methodImage::V2::Mock#upload_imagetakes 2 args whereRealtakes 3, and returns204instead of the responseattr_writerforimage_ref/flavor_refinCompute::ServerYAML.loadin the introspection mockOne caveat on durability
Gemfile.lockis gitignored and the gemspec has an unpinnedadd_development_dependency 'rubocop', so CI re-resolves RuboCop on every run. Combined withAllCops: NewCops: enable, the next RuboCop release activates its new cops immediately and linting goes red again with offenses that are not in the baseline. The baseline here is generated against 1.90.0, which is the current latest and the version CI resolved.Pinning
'rubocop', '~> 1.90.0'in the gemspec would make this hold. Not done here, since it constrains contributors' bundles and is a maintainer call — happy to add it if wanted.There is also a pre-existing config bug left untouched:
.rubocop.ymlusesMetrics/BlockLength: IgnoredMethods:, which RuboCop reports as obsolete (renamed toAllowedMethods) and therefore ignores entirely, so the intendeddescribe/it/contextexemptions never applied. Renaming it would shrink the baseline, but that changes lint policy rather than recording it.