Skip to content

Add SKIP=true installer mode to preserve existing files #893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
justin808 wants to merge 4 commits into
base: main
Choose a base branch
from
+22 −6
Open

Add SKIP=true installer mode to preserve existing files #893

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a9509c2
Add SKIP mode for shakapacker installer conflicts
justin808 Feb 15, 2026
8625eaa
Add changelog entry for SKIP=true installer mode (#893)
justin808 Feb 16, 2026
76498b1
Rename force_option to conflict_option and document FORCE precedence
justin808 Feb 16, 2026
b2aa54a
Retrigger Claude Code Review
justin808 Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ Then run the following to install Shakapacker:
bundle exec rake shakapacker:install
```

Before initiating the installation process, ensure you have committed all the changes. While installing Shakapacker, there might be some conflict between the existing file content and what Shakapacker tries to copy. You can either approve all the prompts for overriding these files or use the `FORCE=true` environment variable before the installation command to force the override without any prompt.
Before initiating the installation process, ensure you have committed all the changes. While installing Shakapacker, there might be some conflict between the existing file content and what Shakapacker tries to copy. You can either approve all the prompts for overriding these files, use `FORCE=true` to overwrite without prompting, or use `SKIP=true` to preserve existing files and only create missing ones.
Copy link
Contributor

@greptile-apps greptile-apps bot Feb 15, 2026

Choose a reason for hiding this comment

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

Consider documenting FORCE+SKIP precedence

The PR description states that FORCE wins when both are set, which is a useful detail for users. Consider adding this to the documentation, e.g.: "If both are set, FORCE=true takes precedence."

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


Shakapacker uses the [`package_json`](https://github.com/shakacode/package_json) gem to handle updating the `package.json` and interacting with the underlying package manager of choice for managing dependencies and running commands; the package manager is managed using the [`packageManager`](https://nodejs.org/api/packages.html#packagemanager) property in the `package.json`, otherwise falling back to the value of `PACKAGE_JSON_FALLBACK_MANAGER` if set or otherwise `npm`.

Expand Down
8 changes: 7 additions & 1 deletion lib/install/binstubs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
force_option = ENV["FORCE"] ? { force: true } : {}
force_option = if ENV["FORCE"]
{ force: true }
elsif ENV["SKIP"]
{ skip: true }
else
{}
end

say "Copying binstubs"
directory "#{__dir__}/bin", "bin", force_option
Expand Down
8 changes: 7 additions & 1 deletion lib/install/template.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@

# Install Shakapacker

force_option = ENV["FORCE"] ? { force: true } : {}
force_option = if ENV["FORCE"]
{ force: true }
elsif ENV["SKIP"]
{ skip: true }
else
{}
end
Copy link
Contributor

@greptile-apps greptile-apps bot Feb 15, 2026

Choose a reason for hiding this comment

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

Variable name is now misleading

The variable force_option can now hold { skip: true }, making the name misleading. Consider renaming to something more general like conflict_option or file_conflict_option to reflect that it handles both force and skip behaviors.

Suggested change
force_option = if ENV["FORCE"]
{ force: true }
elsif ENV["SKIP"]
{ skip: true }
else
{}
end
conflict_option = if ENV["FORCE"]
{ force: true }
elsif ENV["SKIP"]
{ skip: true }
else
{}
end

Note: This would also require updating the three references to force_option on lines 40, 62, and the corresponding variable in lib/install/binstubs.rb (line 10).

Context Used: Context from dashboard - CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +10 to +16
Copy link

@coderabbitai coderabbitai bot Feb 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

ENV["SKIP"] is truthy for any value, including "false" or "0".

ENV["SKIP"] returns a non-nil string for any set value, so SKIP=false bundle exec rake shakapacker:install would still activate skip mode. Consider checking for explicit truthy values to match the documented SKIP=true usage. The same applies to ENV["FORCE"], though that's pre-existing behavior.

Proposed fix 
-conflict_option = if ENV["FORCE"]
+conflict_option = if ENV["FORCE"] == "true"
   { force: true }
-elsif ENV["SKIP"]
+elsif ENV["SKIP"] == "true"
   { skip: true }
 else
   {}
 end
🤖 Prompt for AI Agents 
In `@lib/install/template.rb` around lines 10 - 16, The code currently treats any
set ENV["SKIP"] or ENV["FORCE"] as truthy (so "false" or "0" still activates),
so update the conflict_option logic to test for explicit truthy strings (e.g.
check ENV["SKIP"] && %w[true 1 yes].include?(ENV["SKIP"].to_s.downcase)) and do
the same for ENV["FORCE"] (or extract a small helper like truthy_env?(name)) so
only intended values like "true"/"1"/"yes" enable skip/force in the
conflict_option branch.


# Initialize variables for use throughout the template
# Using instance variable to avoid method definition issues in Rails templates
Expand Down