Skip to content

Conversation

@pftg
Copy link
Collaborator

@pftg pftg commented Sep 13, 2025

Summary by CodeRabbit

  • Tests
    • CI matrix updated to add experimental runs for Ruby 3.5.0-preview1 (Rails 8.1 and edge), added Rails 8.1 to options, and removed Rails 7.0 from the matrix; added/excluded JRuby rows for specific combinations.
  • Style / Refactor
    • Minor whitespace and local-variable renaming in test and assertion code with no behavior changes.

@coderabbitai
Copy link

coderabbitai bot commented Sep 13, 2025

Warning

Rate limit exceeded

@pftg has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 48 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 3861dff and 7d1bcca.

📒 Files selected for processing (1)
  • gemfiles/rails81_gems.rb (1 hunks)

Walkthrough

Updated CI matrix: removed rails70_gems.rb, added rails81_gems.rb, adjusted include/exclude entries (added ruby-version: 3.5.0-preview1 includes and a jruby-head include; excluded jruby-9.4 + rails81_gems.rb non-experimental). Also renamed block parameters in two Ruby files and added one blank line in a test helper (formatting only).

Changes

Cohort / File(s) Summary of Changes
CI workflow matrix
.github/workflows/test.yml
Removed rails70_gems.rb; added rails81_gems.rb to matrix; added matrix.include entries for ruby-version: 3.5.0-preview1 with rails81_gems.rb and edge_gems.rb (both experimental: true); added include for jruby-head with rails80_gems.rb (experimental: true); added an exclude for jruby-9.4 + rails81_gems.rb (experimental: false).
Small formatting
test/support/test_helpers.rb
Inserted a single blank line after include Capybara::Screenshot::Diff::TestDoubles (whitespace change only).
Block parameter renames
lib/capybara/screenshot/diff/area_calculator.rb, lib/capybara_screenshot_diff/screenshot_assertion.rb
Renamed tap/block parameters (itregion in AreaCalculator; itassertion in ScreenshotAssertion). Calls adjusted to use the new parameter names; no logic change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant GH as GitHub Actions
  participant M as Matrix Expander
  participant J_preview_rails81 as Job: 3.5.0-preview1 + rails81_gems
  participant J_preview_edge as Job: 3.5.0-preview1 + edge_gems
  participant J_jruby_head as Job: jruby-head + rails80_gems
  participant T as Test Runner

  Dev->>GH: Push / PR triggers workflow
  GH->>M: Read .github/workflows/test.yml
  Note over M: Matrix now includes preview and jruby-head entries\nand excludes jruby-9.4+rails81 (non-experimental)
  M-->>GH: Expanded matrix
  GH->>J_preview_rails81: Start job (preview + rails81_gems)
  GH->>J_preview_edge: Start job (preview + edge_gems)
  GH->>J_jruby_head: Start job (jruby-head + rails80_gems)
  J_preview_rails81->>T: Install deps & run tests
  J_preview_edge->>T: Install deps & run tests
  J_jruby_head->>T: Install deps & run tests
  T-->>GH: Report job results
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

I nibble YAML lines with glee,
A preview pair hopped in for me.
Rails eighty-one and edge take flight,
A tiny blank line keeps things light. 🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "test: new ruby 3.5" succinctly and accurately reflects the PR's primary purpose—adding/testing Ruby 3.5 in CI—matching changes in .github/workflows/test.yml and the source branch ruby-3.5-bump, so it is concise and on-topic.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 74fc271 and 15ffd87.

📒 Files selected for processing (1)
  • .github/workflows/test.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Functional Test

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/test.yml (1)

9-9: GitHub Actions key should be types, not type

Using type is ignored; review_requested events won’t trigger as intended.

-  pull_request:
-    type: [ opened, synchronize, reopened, review_requested ]
+  pull_request:
+    types: [ opened, synchronize, reopened, review_requested ]
🧹 Nitpick comments (2)
lib/capybara/screenshot/diff/area_calculator.rb (2)

51-51: Rename tap block param to plural for clarity

The block receives an Array of regions; plural reads better and avoids confusion with Region.

-            .tap { |region| region.compact! }
+            .tap { |regions| regions.compact! }

22-23: Fix typo in comment

“to eb relative” → “to be relative”.

-        # and if there is crop then makes absolute coordinates to eb relative to crop top left corner
+        # and if there is crop then makes absolute coordinates to be relative to crop top-left corner
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0de45dd and 3861dff.

📒 Files selected for processing (3)
  • .github/workflows/test.yml (2 hunks)
  • lib/capybara/screenshot/diff/area_calculator.rb (1 hunks)
  • lib/capybara_screenshot_diff/screenshot_assertion.rb (1 hunks)
🔇 Additional comments (3)
lib/capybara_screenshot_diff/screenshot_assertion.rb (1)

21-23: LGTM: clearer tap block param

Renaming it to assertion improves readability; no functional change.

.github/workflows/test.yml (2)

101-101: Add rails81_gems.rb to matrix — looks good

Keeps coverage current with Rails 8.1.


112-114: Exclude jruby-9.4 + rails81 — good guardrail

JRuby 9.4 targets Ruby 3.1; Rails 8.x needs ≥3.2.

@pftg pftg requested a review from Copilot September 13, 2025 15:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Updates the CI matrix to add experimental support for Ruby 3.5.0-preview1, adds Rails 8.1 support, and removes Rails 7.0 from the test matrix.

  • Added Ruby 3.5.0-preview1 experimental testing for Rails 8.1 and edge versions
  • Added Rails 8.1.0.beta1 support via new gemfile
  • Removed Rails 7.0 from CI matrix and excluded JRuby from Rails 8.1 testing

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
.github/workflows/test.yml Updates CI matrix to add Ruby 3.5 experimental runs, Rails 8.1, and removes Rails 7.0
gemfiles/rails81_gems.rb Adds new Rails 8.1.0.beta1 dependency configuration
lib/capybara_screenshot_diff/screenshot_assertion.rb Renames variable from 'it' to 'assertion' for clarity
lib/capybara/screenshot/diff/area_calculator.rb Renames variable from 'it' to 'region' for clarity
test/support/test_helpers.rb Adds whitespace line for formatting

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@pftg pftg merged commit 359b4e4 into master Sep 13, 2025
62 of 64 checks passed
@pftg pftg deleted the ruby-3.5-bump branch September 13, 2025 15:45
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