Fix gemspec's files regexp to exclude Gemfile.lock #856
+1
−1
Conversation
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
WalkthroughThe PR modifies the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shakapacker.gemspec(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: marvinthepa
Repo: shakacode/shakapacker PR: 520
File: spec/shakapacker/utils_manager_spec.rb:68-89
Timestamp: 2024-10-09T10:47:17.620Z
Learning: In `spec/shakapacker/utils_manager_spec.rb`, the code within the context `"when lockfile is in Rails.root, but pwd is different"` is not duplicated for each package manager.
Learnt from: marvinthepa
Repo: shakacode/shakapacker PR: 520
File: lib/shakapacker/utils/manager.rb:19-19
Timestamp: 2024-10-09T10:46:03.499Z
Learning: In `lib/install/template.rb` of the Shakapacker project, calls to `PackageJson.read` are wrapped inside `Dir.chdir(Rails.root)`, ensuring that `package.json` is read from the Rails root directory.
Learnt from: CR
Repo: shakacode/shakapacker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:14:43.766Z
Learning: Run corresponding RSpec tests when changing source files (e.g., when changing `lib/shakapacker/foo.rb`, run `spec/shakapacker/foo_spec.rb`)
Learnt from: marvinthepa
Repo: shakacode/shakapacker PR: 520
File: lib/shakapacker/utils/manager.rb:19-19
Timestamp: 2024-10-09T10:46:03.499Z
Learning: In the Shakapacker project, within `lib/shakapacker/runner.rb`, `PackageJson.read(app_path)` is used to read `package.json` using the application path, ensuring the correct path is used regardless of the current working directory.
Learnt from: CR
Repo: shakacode/shakapacker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:14:43.766Z
Learning: This gem supports both webpack and rspack configurations - test changes with both bundlers when modifying core functionality
Learnt from: marvinthepa
Repo: shakacode/shakapacker PR: 520
File: spec/shakapacker/utils_manager_spec.rb:104-116
Timestamp: 2024-10-09T10:50:16.512Z
Learning: In `spec/shakapacker/utils_manager_spec.rb`, `error_unless_package_manager_is_obvious` is not a test method, so extracting into a shared context may not be helpful.
📚 Learning: 2025-11-25T05:14:43.766Z
Learnt from: CR
Repo: shakacode/shakapacker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:14:43.766Z
Learning: Run corresponding RSpec tests when changing source files (e.g., when changing `lib/shakapacker/foo.rb`, run `spec/shakapacker/foo_spec.rb`)
Applied to files:
shakapacker.gemspec
📚 Learning: 2025-03-05T21:39:32.978Z
Learnt from: shoeyn
Repo: shakacode/shakapacker PR: 552
File: CHANGELOG.md:13-16
Timestamp: 2025-03-05T21:39:32.978Z
Learning: The CHANGELOG.md in shakapacker uses dash-style bullets (-) for list items, not asterisks (*).
Applied to files:
shakapacker.gemspec
📚 Learning: 2024-10-09T10:47:17.620Z
Learnt from: marvinthepa
Repo: shakacode/shakapacker PR: 520
File: spec/shakapacker/utils_manager_spec.rb:68-89
Timestamp: 2024-10-09T10:47:17.620Z
Learning: In `spec/shakapacker/utils_manager_spec.rb`, the code within the context `"when lockfile is in Rails.root, but pwd is different"` is not duplicated for each package manager.
Applied to files:
shakapacker.gemspec
📚 Learning: 2024-10-09T10:46:03.499Z
Learnt from: marvinthepa
Repo: shakacode/shakapacker PR: 520
File: lib/shakapacker/utils/manager.rb:19-19
Timestamp: 2024-10-09T10:46:03.499Z
Learning: In `lib/install/template.rb` of the Shakapacker project, calls to `PackageJson.read` are wrapped inside `Dir.chdir(Rails.root)`, ensuring that `package.json` is read from the Rails root directory.
Applied to files:
shakapacker.gemspec
📚 Learning: 2025-11-25T05:14:43.766Z
Learnt from: CR
Repo: shakacode/shakapacker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:14:43.766Z
Learning: Always run `bundle exec rubocop` before committing Ruby changes
Applied to files:
shakapacker.gemspec
📚 Learning: 2025-11-25T05:14:43.766Z
Learnt from: CR
Repo: shakacode/shakapacker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:14:43.766Z
Learning: Run the full test suite with `bundle exec rspec` before pushing
Applied to files:
shakapacker.gemspec
📚 Learning: 2025-11-25T05:14:43.766Z
Learnt from: CR
Repo: shakacode/shakapacker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:14:43.766Z
Learning: Always use `bundle exec` prefix when running Ruby commands (rubocop, rspec, rake, etc.)
Applied to files:
shakapacker.gemspec
Comment on lines
31
to
33
| s.files = `git ls-files -z`.split("\x0").reject { |f| | ||
| f.match(%r{^(test|spec|features|tmp|node_modules|packages|coverage|Gemfile.lock|rakelib)/}) | ||
| f.match(%r{^(test|spec|features|tmp|node_modules|packages|coverage|Gemfile.lock|rakelib)($|/)}) | ||
| } + Dir.glob("sig/**/*.rbs") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify test and documentation changes for Gemfile.lock exclusion
# 1. Search for tests related to gemspec files exclusion
echo "=== Searching for gemspec-related tests ==="
find . -name "*.rb" -type f \( -path "*/spec/*" -o -path "*/test/*" \) 2>/dev/null | xargs grep -l "gemspec\|\.files\|Gemfile\.lock" 2>/dev/null | head -20
# 2. Check CHANGELOG for recent updates
echo -e "\n=== Checking CHANGELOG file ==="
if [ -f "CHANGELOG.md" ]; then
head -50 CHANGELOG.md
elif [ -f "CHANGELOG" ]; then
head -50 CHANGELOG
else
echo "No CHANGELOG.md or CHANGELOG found"
fi
# 3. Check if Gemfile.lock is tracked by git
echo -e "\n=== Checking if Gemfile.lock is tracked by git ==="
git ls-files 2>/dev/null | grep "^Gemfile.lock$" && echo "Gemfile.lock is tracked" || echo "Gemfile.lock not tracked"
# 4. Check the actual shakapacker.gemspec file at lines 31-33
echo -e "\n=== Verifying shakapacker.gemspec lines 31-33 ==="
sed -n '31,33p' shakapacker.gemspecRepository: shakacode/shakapacker
Length of output: 4822
🏁 Script executed:
#!/bin/bash
# More targeted search for Gemfile.lock in CHANGELOG and tests
echo "=== Searching CHANGELOG for Gemfile.lock entry ==="
grep -n "Gemfile.lock" CHANGELOG.md 2>/dev/null | head -20
echo -e "\n=== Searching spec files for Gemfile.lock related tests ==="
find ./spec -name "*.rb" -type f 2>/dev/null -exec grep -l "Gemfile.lock\|s\.files\|gemspec" {} \;
echo -e "\n=== Searching for files exclusion tests in detail ==="
find ./spec -name "*gemspec*" -o -name "*files*" 2>/dev/null | head -10
echo -e "\n=== Check git log for recent changes to shakapacker.gemspec ==="
git log --oneline -20 -- shakapacker.gemspec 2>/dev/nullRepository: shakacode/shakapacker
Length of output: 373
🏁 Script executed:
#!/bin/bash
# Check the specific commit that fixed the gemspec
echo "=== Details of commit 1fc855d ==="
git show --stat 1fc855d 2>/dev/null | head -30
echo -e "\n=== Check if CHANGELOG.md was mentioned in this commit ==="
git show 1fc855d 2>/dev/null | grep -i "changelog" || echo "No CHANGELOG mention in commit"
echo -e "\n=== List all files changed in commit 1fc855d ==="
git show --name-only --pretty=format: 1fc855d 2>/dev/nullRepository: shakacode/shakapacker
Length of output: 22275
🏁 Script executed:
#!/bin/bash
# Final verification: search entire CHANGELOG for Gemfile.lock entry
echo "=== Entire CHANGELOG search for Gemfile.lock ==="
grep -n "Gemfile.lock\|gemspec.*files\|files.*exclusion" CHANGELOG.md 2>/dev/null | head -20
echo -e "\n=== Count of lines in CHANGELOG.md ==="
wc -l CHANGELOG.md
echo -e "\n=== Search for any gemspec-related tests ==="
find ./spec -name "*.rb" -type f -exec grep -l "\.files\|gemspec\|exclude" {} \; 2>/dev/nullRepository: shakacode/shakapacker
Length of output: 271
Add test and CHANGELOG entry for Gemfile.lock exclusion.
The regex pattern correctly excludes Gemfile.lock. The ($|/) anchor ensures both exact filenames (via $) and directory paths (via /) are matched, preventing false positives like Gemfile.lock.bak.
However, add a test in the spec suite to verify that Gemfile.lock is excluded from s.files in the gemspec, and add a CHANGELOG entry documenting this change under the appropriate version section.
🤖 Prompt for AI Agents
In shakapacker.gemspec around lines 31–33, the reviewer asks to add a spec test
that verifies Gemfile.lock is excluded from s.files and to add a CHANGELOG entry
documenting this behavior; write a unit test in the gem's spec suite that loads
or evaluates the gemspec, computes s.files (or invokes the same git/Dir logic),
and asserts that "Gemfile.lock" is not present, place the test alongside other
gemspec/specs, and update the CHANGELOG.md (or CHANGELOG) under the current
unreleased or appropriate version section with a short entry noting that
Gemfile.lock is excluded from s.files so the gem packaging will not include it.
Member
|
@claude review this PR. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Gemfile.lockis being unexpectedly published as part of the gem.No issue technically as it is ignored when installing the gem, but in our case it is triggering vulnerability alerts due to outdated pinned versions during the docker image scan.
Pull Request checklist
Add/update test to cover these changesUpdate documentationUpdate CHANGELOG fileSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.