Skip to content

Fix typo by copilot review findings#9471

Merged
hsbt merged 4 commits intomasterfrom
fix-copilot-review-findings
Apr 8, 2026
Merged

Fix typo by copilot review findings#9471
hsbt merged 4 commits intomasterfrom
fix-copilot-review-findings

Conversation

@hsbt
Copy link
Copy Markdown
Member

@hsbt hsbt commented Apr 8, 2026

What is your fix for the problem, implemented in this PR?

Fixed typo and incorrectly test assertion words.

Make sure the following tasks are checked

hsbt and others added 2 commits April 8, 2026 13:25
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The Rust function hello("world") returns "Hello world, from Rust!" but
the Ruby test templates expected "Hello earth, from Rust!", causing
generated tests to fail immediately after bundle gem --ext=rust.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

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

This PR updates Bundler’s generated newgem templates and documentation to correct a few typos, primarily around the Rust extension “hello” string and some wording in comments/manpages.

Changes:

  • Update Rust newgem template expectations from “Hello earth…” to “Hello world…”.
  • Fix typos in an internal comment (reaedread, a threadsa thread).
  • Fix a typo in the bundle-config manpage (fetchigfetching) in both .ronn and generated .1.

Reviewed changes

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

Show a summary per file
File Description
bundler/lib/bundler/templates/newgem/test/minitest/test_newgem.rb.tt Fixes expected Rust greeting string in generated Minitest template.
bundler/lib/bundler/templates/newgem/spec/newgem_spec.rb.tt Fixes expected Rust greeting string in generated RSpec template (also highlights a matcher correctness issue).
bundler/lib/bundler/source/rubygems.rb Fixes typos in a concurrency-related comment near gem installer caching.
bundler/lib/bundler/man/bundle-config.1.ronn Fixes a typo in the api_request_size documentation.
bundler/lib/bundler/man/bundle-config.1 Mirrors the .ronn typo fix in the generated manpage output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

result = <%= config[:constant_name] %>.hello("world")

expect(result).to be("Hello earth, from Rust!")
expect(result).to be("Hello world, from Rust!")
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

In this RSpec template, be("Hello world, from Rust!") checks object identity (equal?) and will generally fail for newly-allocated strings. Use a value-equality matcher like eq/eql here so the generated spec passes reliably.

Suggested change
expect(result).to be("Hello world, from Rust!")
expect(result).to eq("Hello world, from Rust!")

Copilot uses AI. Check for mistakes.
Comment thread bundler/lib/bundler/source/rubygems.rb Outdated
# We are using a mutex to reaed and write from/to the hash.
# We are using a mutex to read and write from/to the hash.
# The reason this double synchronization was added is for performance
# and lock the mutex for the shortest possible amount of time. Otherwise,
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

Minor grammar nit in this comment: "is for performance and lock the mutex" reads like a typo; consider changing to "is for performance and to lock the mutex ..." for clarity.

Suggested change
# and lock the mutex for the shortest possible amount of time. Otherwise,
# and to lock the mutex for the shortest possible amount of time. Otherwise,

Copilot uses AI. Check for mistakes.
hsbt and others added 2 commits April 8, 2026 14:15
be checks object identity which will fail for newly-allocated strings.
eq checks value equality and is the correct matcher here.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Change "and lock the mutex" to "and to lock the mutex" for correct
parallel infinitive structure.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@hsbt hsbt merged commit 94f9267 into master Apr 8, 2026
97 checks passed
@hsbt hsbt deleted the fix-copilot-review-findings branch April 8, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants