Add --skip-install option to fedify init#776
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a ChangesSkip-Install Flag Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGES.md`:
- Around line 270-274: The CHANGES.md entry for the `--skip-install` option to
`fedify init` currently only includes the issue link `[[`#720`]]`; update this
changelog line to also include the PR number (e.g. `[[PR#<number>]]`) and the
author attribution (your name) in the same format used elsewhere in CHANGES.md
so it reads: description, then issue link, PR link, and author name; edit the
existing `--skip-install` entry text to append the PR reference and author
attribution to match the repository’s changelog convention.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 73163226-a947-4e5d-ae20-f63ef9e6f85e
📒 Files selected for processing (11)
CHANGES.mddocs/cli.mdpackages/init/src/action/configs.test.tspackages/init/src/action/mod.tspackages/init/src/action/notice.tspackages/init/src/action/patch.test.tspackages/init/src/action/utils.tspackages/init/src/command.tspackages/init/src/package.test.tspackages/init/src/skip-install.test.tspackages/init/src/webframeworks.test.ts
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a --skip-install option to the fedify init command, allowing users to skip the automatic dependency installation step. This is useful for CI environments, monorepos, or manual inspection. The changes include updates to the CLI documentation, the addition of the flag in the command configuration, logic to conditionally skip installation in the initialization pipeline, and a new notice to inform users how to install dependencies manually. Feedback suggests reordering the final notices so that the instruction to install dependencies appears before the instructions on how to run the project, ensuring a more logical flow for the user.
| tap(noticeHowToRun), | ||
| tap(unless(isDry, when(isSkipInstall, noticeSkippedInstall))), |
There was a problem hiding this comment.
The notice about skipped dependency installation should ideally appear before the "how to run" instructions. This provides a better user experience by informing the user that they need to perform an installation step before they attempt to run the project using the commands shown in the next step.
| tap(noticeHowToRun), | |
| tap(unless(isDry, when(isSkipInstall, noticeSkippedInstall))), | |
| tap(unless(isDry, when(isSkipInstall, noticeSkippedInstall))), | |
| tap(noticeHowToRun), |
|
cc @2chanhaeng |
|
Hi there! Thank you so much for your interest in contributing to our package. When initializing an app with the |
Codecov Report❌ Patch coverage is
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
2chanhaeng
left a comment
2chanhaeng
left a comment
There was a problem hiding this comment.
Thanks for your contributions, again!
First, sorry for late. I've been sick for the past few days and still haven't fully recovered, so my review is late. I should have at least let you know. I apologize if you've been waiting a long time.
Before getting into the review, I recommend that in the future, you leave a comment on the review explaining how you applied the feedback along with the relevant commit if you accepted it, or the reasoning behind your decision if you rejected it. This will help the reviewer review your revisions more quickly.
The flow you added looks more complex than I thought. Since InitCommandOptions has a skipInstall prop, I think it would be enough to just add the related command in WebFrameworkDescription.init when skipInstall is present. I'm really sorry, but could you revert 39bb2df and then only add the part that handles the skipInstall argument in packages/init/src/webframeworks/next.ts? It should be sufficient to add the skipInstall: boolean param to getNextInitCommand and then append "--skip-install" if that argument is true.
Also, this repo doesn't ban contributions using AI agents, but you should add trailers in the commit messages if you used any AI agents. You can find explanations about trailers in AI_POLICY.md. Are you sure you didn't use any AI agents at all?
Assisted-by: Claude Code:claude-opus-4-7
|
Thanks for the feedback. I've simplified the approach by removing the withSkipInstallArgs logic and now directly passing skipInstall: boolean to getNextInitCommand in next.ts (0cccc38). |
2chanhaeng
left a comment
2chanhaeng
left a comment
There was a problem hiding this comment.
Thanks for your contributions, and I like your work! I have some suggestions to make your contribution even better. Please consider applying these reviews.
| when(isDry, handleDryRun), | ||
| unless(isDry, handleHydRun), | ||
| tap(recommendConfigEnv), | ||
| tap(unless(isDry, when(isSkipInstall, noticeSkippedInstall))), |
There was a problem hiding this comment.
It seems more sense to place when(isSkipInstall, noticeSkippedInstall) inside handleHydRun. Or is there a specific reason you intended for it to be here?
| ) => | ||
| printMessage` | ||
| Dependencies were not installed. Run ${ | ||
| text(`${packageManager} install`) |
There was a problem hiding this comment.
The text function outputs its arguments so that they appear as plain text. I think commandLine is better for this.
3 participants
Closes #720.
fedify initalways ran the package manager's install command afterscaffolding, with no way to skip it. This is disruptive in CI
pipelines that install from a fixed lockfile in a separate step, in
monorepo workspaces that install from the root, and when developers
want to inspect the generated files before installing.
Adds
--skip-installto theinitOptionsschema and threads anisSkipInstallpredicate through the action pipeline soinstallDependenciesis bypassed when the flag is set. When the runis not in dry-run mode,
noticeSkippedInstallprints the exact installcommand for the selected package manager.
Verification
mise run checkmise run test