feat: add v1.2.0 features for post-framework-starter workflow#7
feat: add v1.2.0 features for post-framework-starter workflow#7
Conversation
- Auto-detect language from project marker files (go.mod, package.json, Cargo.toml, composer.json, Gemfile, pubspec.yaml, pom.xml, etc.) when -l is not provided; prints detected language to output - Add --no-license flag to skip LICENSE creation without quiet mode - Add --no-readme flag to skip README.md creation without quiet mode - Add --post-framework flag (implies --no-license --no-readme, enables language auto-detection and existing-branch detection) - Auto-detect active branch from .git/HEAD when --branch is not explicitly set, honouring branches created by framework starters - Add DetectLanguage() in internal/files/gitignore.go - Add DetectCurrentBranch() in internal/repo/repo.go - Add tests: internal/files/detect_test.go (13 cases), internal/repo/repo_branch_test.go (5 cases) - Add Makefile targets: test-smoke, test-clean - Add planning/feature-planning.md for v1.2.0 and v1.3.0 roadmap - Update dry-run output to reflect all new flags and detected values
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds marker-based language auto-detection, Git branch auto-detection and branch-aware repo init/push, new CLI flags (--post-framework, --no-license, --no-readme), enhanced dry-run/resolved-path displays, Makefile smoke/cleanup targets, and tests for language and branch detection. Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as cmd/root.go
participant FS as internal/files
participant Git as internal/repo
User->>CLI: invoke CLI (flags may include --post-framework, -l, -b)
CLI->>CLI: Parse flags (post-framework implies no-license/no-readme)
rect rgba(100,150,200,0.5)
alt Language not provided
CLI->>FS: DetectLanguage(resolvedDir)
FS-->>CLI: language or ""
end
end
rect rgba(150,100,200,0.5)
alt Branch not provided
CLI->>Git: DetectCurrentBranch(resolvedDir)
Git-->>CLI: branch or ""
end
end
CLI->>CLI: resolveDir(dir), determine effectiveBranch, assemble dry-run display
CLI-->>User: show resolved path, repo name, language (auto/explicit/none), branch, license/readme flags, push target
alt User proceeds with push
CLI->>Git: InitGitRepo(dir, effectiveBranch)
CLI->>Git: createRemoteAndPush(dir, repoName, pushBranch)
Git-->>CLI: success/failure
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cmd/root.go (1)
207-208: Consider renaming toresolveDirfor clarity.The function name
resolvDiris missing an 'e'. While not a functional issue,resolveDirwould be more readable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/root.go` around lines 207 - 208, Rename the function identifier resolvDir to resolveDir and update its comment to match; then find and update every call site and any references (including tests, method receivers, and documentation/comments) to use resolveDir so compilation and behavior remain unchanged (the function body and signature stay the same).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/root.go`:
- Around line 88-95: Dry-run currently sets detectedLang only when postFramework
is true which causes effectiveLang to be empty during dry-run even though
ensureGitignore later auto-detects language; update the dry-run logic to mirror
ensureGitignore by running files.DetectLanguage(dir) whenever language is empty
(regardless of postFramework) so detectedLang/effectiveLang reflect the same
auto-detection behavior as ensureGitignore (referencing detectedLang, language,
postFramework, effectiveLang, and ensureGitignore to locate the code).
In `@internal/files/gitignore.go`:
- Around line 13-28: languageMarkers currently maps composer.json to "PHP",
which fails because PHP.gitignore isn't available; update the mapping to remove
or remap composer.json. Locate the languageMarkers slice and either delete the
entry {files: []string{"composer.json"}, language: "PHP"} or change its language
value to an existing template name (for example "Composer" or another valid
github/gitignore entry) so FetchGitignore will not request PHP.gitignore; ensure
only the composer.json file string and its language value are modified in the
languageMarkers variable.
---
Nitpick comments:
In `@cmd/root.go`:
- Around line 207-208: Rename the function identifier resolvDir to resolveDir
and update its comment to match; then find and update every call site and any
references (including tests, method receivers, and documentation/comments) to
use resolveDir so compilation and behavior remain unchanged (the function body
and signature stay the same).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0826baa6-e524-4d26-bbb0-61f7977c3c40
📒 Files selected for processing (6)
Makefilecmd/root.gointernal/files/detect_test.gointernal/files/gitignore.gointernal/repo/repo.gointernal/repo/repo_branch_test.go
- Rename resolvDir to resolveDir (typo fix) - Fix dry-run language detection to always auto-detect when -l is omitted, not only when --post-framework is set; aligns dry-run output with actual ensureGitignore behaviour - Fix composer.json marker: map to "Composer" not "PHP" since PHP.gitignore does not exist in github/gitignore; update detect_test.go expected value accordingly
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/root.go`:
- Line 204: The help text for the postFramework flag (set via
rootCmd.PersistentFlags().BoolVar(&postFramework, "post-framework", ...)) is
misleading about auto-detection and branch detection; update the flag
description to remove “enables auto-detection” and instead clearly state that it
optimises behavior for projects created by framework starters and implies
--no-license and --no-readme, while leaving language/branch auto-detection
behavior unchanged. Locate the BoolVar call for postFramework and replace the
description string with wording that reflects only the implied flags and the
intended optimisation without claiming additional detection features.
- Around line 76-81: The code mutates flag-bound variables noLicense and
noReadme inside rootCmd.Run when postFramework is true; instead derive local
effective booleans and leave the bound vars untouched. In rootCmd.Run (where
postFramework is checked) replace assignments like "noLicense = true" and
"noReadme = true" with local variables, e.g. effectiveNoLicense := noLicense ||
postFramework and effectiveNoReadme := noReadme || postFramework, and use these
effective* variables everywhere downstream (in LICENSE/README creation logic and
any other checks) so flag state from BoolVar() remains immutable across
Execute() calls.
- Around line 379-388: The dry-run message can diverge because
effectiveBranch(dir) autodetects .git/HEAD even for repos we just initialized;
change ensureGitRepo to return (or set) a flag indicating the repo was newly
created (e.g., ensureGitRepo -> ensureGitRepo(dir) (created bool, err error) or
set repoWasNewlyCreated), then update callers and modify effectiveBranch to
accept that flag (e.g., effectiveBranch(dir string, repoNew bool) string) and
only call repo.DetectCurrentBranch(dir) when repoNew is false and branchExplicit
is false; keep the default branch variable (branch) otherwise so the dry-run
shows the promised target.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 942b167a-5181-4060-a635-02ae7f4e0e50
📒 Files selected for processing (3)
cmd/root.gointernal/files/detect_test.gointernal/files/gitignore.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/files/gitignore.go
- internal/files/detect_test.go
- Stop mutating Cobra-bound flag vars: replace the postFramework block that overwrote noLicense/noReadme with derived locals effNoLicense and effNoReadme (|| postFramework); thread them as parameters through run(), ensureLicense(), and ensureReadme() to avoid corrupted state across multiple in-process Execute() calls - Fix --post-framework help text: remove "enables auto-detection" since language and branch auto-detection are unconditional, not gated by this flag - Fix branch divergence on newly-initialised repos: InitGitRepo now accepts a branch parameter and passes -b to git init, making the initial branch deterministic regardless of system init.defaultBranch; effectiveBranch() is resolved once before ensureGitRepo() and passed through to both init and push, so dry-run output always matches the actual run; update TestInitGitRepo to pass branch argument
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
docs/README.md (1)
117-118:⚠️ Potential issue | 🟡 MinorRemove the “enables auto-detection” wording here as well.
This now overstates what
--post-frameworkdoes. Language and branch auto-detection happen without that flag, so the docs/changelog should only describe the implied--no-license/--no-readmebehavior.Also applies to: 294-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/README.md` around lines 117 - 118, Update the README description for the --post-framework flag: remove the phrase "enables auto-detection" so the entry only notes the implied --no-license and --no-readme behavior; apply the same removal to the duplicate description further down (the other occurrence of --post-framework) so both places only state the implied options without claiming auto-detection.cmd/root.go (1)
96-103:⚠️ Potential issue | 🟠 MajorBranch auto-detection still leaks across repeated
Execute()calls.This new path depends on Cobra’s
Changed("branch")bit and the boundbranchvariable. On a reused command tree, a previous--branch developrun can leave both stale, so a later invocation without--branchskips auto-detection and still pushesdevelop.The safest fix is the same as for the earlier flag-state bug: construct a fresh root command per invocation, or explicitly reset the branch flag/value before each parse.
Does spf13/cobra / pflag preserve a flag's Changed state and bound variable value across repeated Execute() calls on the same command without ResetFlags?Also applies to: 152-154, 177-179, 387-394
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/root.go` around lines 96 - 103, The branch auto-detection leaks state across repeated command.Execute() calls because cobra/pflag preserves a flag's Changed bit and the bound variable value; before parsing/executing the command ensure you clear the bound variable and reset the flag's Changed state (e.g., set the package-level or command-scoped variable branch = "" and call cmd.Flags().Lookup("branch").Changed = false) or, alternatively, construct a fresh root command per invocation; update the logic around cmd.Flags().Changed("branch") and the branch variable (and replicate the same reset for the other occurrences you noted) so auto-detection runs when no explicit flag was provided in the current invocation.README.md (1)
117-118:⚠️ Potential issue | 🟡 MinorDrop the “enables auto-detection” claim here too.
--post-frameworkno longer changes whether language/branch auto-detection runs; it only implies--no-licenseand--no-readme. The option table and v1.2.0 changelog still describe the old behavior.Also applies to: 294-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 117 - 118, The README still claims that --post-framework "enables auto-detection"; update the option description for --post-framework to remove "enables auto-detection" so it only states it implies --no-license and --no-readme, and also update the v1.2.0 changelog and the option table references (the other occurrence around the changelog lines) to remove the same inaccurate phrase so all mentions reflect that --post-framework no longer affects language/branch auto-detection.
🧹 Nitpick comments (1)
internal/repo/repo_test.go (1)
12-18: Assert the initialized branch too.This update is about deterministic branch creation, but the test still only proves that
.gitwas created. It would pass even ifInitGitRepoignored the"main"argument and used Git’s default branch instead.Suggested test tightening
if _, err := os.Stat(filepath.Join(tmpDir, ".git")); err != nil { t.Fatalf("expected .git directory to exist, got error: %v", err) } + if got := DetectCurrentBranch(tmpDir); got != "main" { + t.Fatalf("expected branch %q, got %q", "main", got) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/repo/repo_test.go` around lines 12 - 18, The test currently only verifies that .git exists but not which branch was created; update the test that calls InitGitRepo(tmpDir, "main") to also assert the repo's current branch is "main" by invoking Git to read the HEAD (e.g., run git -C tmpDir rev-parse --abbrev-ref HEAD or git symbolic-ref --short HEAD) and compare the trimmed output to "main", failing the test if it differs; keep the existing .git existence check and use the same tmpDir/InitGitRepo call to locate the repo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/README.md`:
- Line 299: Update the README line that currently references "github/gitignore"
(the diff text: Fixed `composer.json` marker mapping from `PHP` to `Composer`
(PHP.gitignore does not exist in github/gitignore)) to use the proper
project/platform name "GitHub/gitignore" (capitalize "GitHub"); ensure the
parenthetical and any other occurrences in the same sentence match the corrected
capitalization.
In `@internal/repo/repo.go`:
- Around line 43-55: DetectCurrentBranch currently assumes .git is a directory;
update it to resolve .git indirection by checking if filepath.Join(dir, ".git")
is a file and, if so, read that file for a "gitdir: <path>" line, resolve that
path relative to dir if it's relative, then use the resolved gitdir/HEAD path
(instead of dir/.git/HEAD) when reading HEAD; alternatively you may invoke git
(e.g., run "git symbolic-ref --short HEAD" in dir) and return its output. Ensure
this change is applied inside DetectCurrentBranch so callers like CommitAndPush
get the actual checked-out branch.
In `@README.md`:
- Line 299: Update the changelog note that currently reads "github/gitignore" to
use the proper capitalization "GitHub/gitignore" in the line mentioning the
fixed `composer.json` marker mapping (the string containing "Fixed
`composer.json` marker mapping from `PHP` to `Composer` (PHP.gitignore does not
exist in github/gitignore)"). Ensure only the capitalization is changed and the
rest of the sentence remains unchanged.
- Around line 107-123: The fenced code blocks in README.md showing the CLI
options (the blocks around the option list at the shown diff and the similar
block at lines 266-272) lack a language identifier and trigger MD040; add a
language tag (e.g., change the opening triple backticks to ```text) for those
fenced blocks so markdownlint-cli2 stops flagging them and the blocks render
correctly.
---
Duplicate comments:
In `@cmd/root.go`:
- Around line 96-103: The branch auto-detection leaks state across repeated
command.Execute() calls because cobra/pflag preserves a flag's Changed bit and
the bound variable value; before parsing/executing the command ensure you clear
the bound variable and reset the flag's Changed state (e.g., set the
package-level or command-scoped variable branch = "" and call
cmd.Flags().Lookup("branch").Changed = false) or, alternatively, construct a
fresh root command per invocation; update the logic around
cmd.Flags().Changed("branch") and the branch variable (and replicate the same
reset for the other occurrences you noted) so auto-detection runs when no
explicit flag was provided in the current invocation.
In `@docs/README.md`:
- Around line 117-118: Update the README description for the --post-framework
flag: remove the phrase "enables auto-detection" so the entry only notes the
implied --no-license and --no-readme behavior; apply the same removal to the
duplicate description further down (the other occurrence of --post-framework) so
both places only state the implied options without claiming auto-detection.
In `@README.md`:
- Around line 117-118: The README still claims that --post-framework "enables
auto-detection"; update the option description for --post-framework to remove
"enables auto-detection" so it only states it implies --no-license and
--no-readme, and also update the v1.2.0 changelog and the option table
references (the other occurrence around the changelog lines) to remove the same
inaccurate phrase so all mentions reflect that --post-framework no longer
affects language/branch auto-detection.
---
Nitpick comments:
In `@internal/repo/repo_test.go`:
- Around line 12-18: The test currently only verifies that .git exists but not
which branch was created; update the test that calls InitGitRepo(tmpDir, "main")
to also assert the repo's current branch is "main" by invoking Git to read the
HEAD (e.g., run git -C tmpDir rev-parse --abbrev-ref HEAD or git symbolic-ref
--short HEAD) and compare the trimmed output to "main", failing the test if it
differs; keep the existing .git existence check and use the same
tmpDir/InitGitRepo call to locate the repo.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b6d0c59-2aa1-439d-be9c-b493867b78d6
📒 Files selected for processing (5)
README.mdcmd/root.godocs/README.mdinternal/repo/repo.gointernal/repo/repo_test.go
- Fix repo_commit_test.go: pass "main" as branch to InitGitRepo,
missed when the signature was updated from InitGitRepo(dir) to
InitGitRepo(dir, branch) in the previous commit
fix: address five code review issues in v1.2.0
- Fix branchExplicit stale state across Execute() calls: remove
package-level branchExplicit var; derive branchWasSet as a local
in Run() via cmd.Flags().Changed("branch") and thread it through
run(), effectiveBranch(), and the dry-run path — same pattern as
effNoLicense/effNoReadme
- Fix DetectCurrentBranch for worktree/submodule repos: add
resolveGitDir() which follows the "gitdir: <path>" pointer when
.git is a file rather than a directory; DetectCurrentBranch now
calls resolveGitDir() before reading HEAD
- Add TestDetectCurrentBranch_WorktreeFile to verify .git file
indirection is resolved correctly
- Assert branch name in TestInitGitRepo via DetectCurrentBranch so
the test proves the -b flag was honoured, not just that .git exists
- Remove "enables auto-detection" from --post-framework help text,
options table, and v1.2.0 changelog in README.md and docs/README.md
- Capitalise GitHub/gitignore in both READMEs
- Add "text" language identifier to bare fenced code blocks in both
READMEs to fix markdownlint MD040 warnings
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/repo/repo_branch_test.go`:
- Around line 81-82: The test's error formatting in repo_branch_test.go uses
t.Errorf("DetectCurrentBranch() = %q, want %q", "feature", got) with the
arguments reversed; update the call in the DetectCurrentBranch test to pass the
observed value first and the expected value second (i.e., use got then
"feature") so the error message prints "got" then "want" correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f656d18-9e94-4324-bf3b-f2246ca027a9
📒 Files selected for processing (7)
README.mdcmd/root.godocs/README.mdinternal/repo/repo.gointernal/repo/repo_branch_test.gointernal/repo/repo_commit_test.gointernal/repo/repo_test.go
t.Errorf format is "got %q, want %q" but args were reversed — "feature" (want) was passed first and got second, producing a misleading failure message on test failure
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Closes #
📑 Description
✅ Checks
ℹ Additional Information
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
Documentation