🚸 Execute go mod tidy after basic creation
#123
Conversation
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant createBasic
participant ShellCmd
User->>createBasic: invoke createBasic(modName, dir)
createBasic->>ShellCmd: runCmd(execCommand "go" "mod" "init" modName) [cwd=dir]
alt init error
ShellCmd-->>createBasic: error
createBasic-->>User: return error
else init ok
createBasic->>ShellCmd: runCmd(execCommand "go" "mod" "tidy") [cwd=dir]
alt tidy error
ShellCmd-->>createBasic: error
createBasic-->>User: return error
else tidy ok
createBasic-->>User: return success
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 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)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🪛 GitHub Actions: golangci-lintcmd/new.go[error] 92-92: File is not properly formatted (gci) 🪛 GitHub Check: lintcmd/new.go[failure] 96-96: [failure] 93-93: [failure] 92-92: ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting 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
🧹 Outside diff range and nitpick comments (1)
cmd/new.go (1)
81-91: Consider enhancing command execution robustnessA few suggestions to improve the implementation:
- Add timeouts to prevent hanging on slow network connections
- Provide more descriptive error messages
- Update the success message to mention module installation
Example implementation with timeout:
func createBasic(projectPath, modName string) (err error) { // create main.go if err = createFile(fmt.Sprintf("%s%cmain.go", projectPath, os.PathSeparator), newBasicTemplate); err != nil { return } - if err = runCmd(execCommand("go", "mod", "init", modName)); err != nil{ + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + initCmd := execCommand("go", "mod", "init", modName) + initCmd.WaitDelay = ctx.Done() + if err = runCmd(initCmd); err != nil { + return fmt.Errorf("failed to initialize module: %w", err) } - installModules := execCommand("go", "mod", "tidy") + installModules := execCommand("go", "mod", "tidy") + installModules.WaitDelay = ctx.Done() installModules.Dir = projectPath - if err = runCmd(installModules); err != nil{ + if err = runCmd(installModules); err != nil { + return fmt.Errorf("failed to install dependencies: %w", err) } return }Also consider updating the success message template to mention module installation:
newSuccessTemplate = ` Scaffolding project in %s (module %s) + ✓ Created project structure + ✓ Initialized Go module + ✓ Installed dependencies Done. Now run: cd %s fiber dev ✨ Done in %s. `
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
cmd/new.go(1 hunks)
🔇 Additional comments (2)
cmd/new.go (2)
81-83: LGTM: Proper error handling for module initialization
The error handling for go mod init is well-implemented and follows Go's idiomatic patterns.
81-91: Verify error handling and cleanup
Let's verify that errors are properly propagated and the project directory is cleaned up on failure.
✅ Verification successful
Let me gather more information about the error handling in the parent function.
Error handling and cleanup are properly implemented
The code has robust error handling and cleanup mechanisms:
- The parent function has a deferred cleanup that removes the project directory on any error
runCmdproperly handles command execution with stderr/stdout pipes and proper cleanup- All critical operations (mkdir, chdir, go mod commands) have error checks
- The project directory is cleaned up if any step in the initialization fails
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify error handling and cleanup implementation
# Check if the defer cleanup is triggered on errors
rg -A 5 "defer func\(\)" cmd/
# Check if the parent function handles errors from module commands
rg "if err = create\w+\(" cmd/
Length of output: 4780
Script:
#!/bin/bash
# Check the parent function implementation and error handling
rg -B 5 -A 10 "createProject\(" cmd/
# Check the runCmd implementation
rg -B 5 -A 10 "func runCmd\(" cmd/
Length of output: 1575
valentimarco
changed the title
go mod tidy after basic creationgo mod tidy after basic creation
gaby
requested review from
efectn,
gaby and
sixcolors
and removed request for
a team
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cmd/new.go`:
- Around line 89-100: The function uses bare returns but doesn't have named
return values; change the three bare "return" statements to return the error
from the failing call (e.g., after "if err := runCmd(...); err != nil { return
err }" and similarly for the "if err := runCmd(installModules); err != nil {
return err }"), and make the final successful exit return nil instead of a bare
return so the function returns an error type consistently; look for runCmd,
execCommand and the surrounding createBasic function to update the returns.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/new.go
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@copilot fix the issues in this PR |
There was a problem hiding this comment.
Pull request overview
Adds an automatic go mod tidy step to the fiber new basic template flow so newly generated projects have a complete, runnable go.mod/go.sum out of the box (fixes #122).
Changes:
- Replaces the single
go mod initcall with explicit error handling. - Executes
go mod tidyafter initializing the module during basic project creation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err := runCmd(execCommand("go", "mod", "init", modName)); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| //Execute go mod tidy in the project directory | ||
| installModules := execCommand("go", "mod", "tidy") | ||
| installModules.Dir = fmt.Sprintf("%s%c", projectPath, os.PathSeparator) |
There was a problem hiding this comment.
createBasic runs go mod init without setting Cmd.Dir, but sets Cmd.Dir for go mod tidy. Since the function already receives projectPath, it would be more robust/consistent to set Dir for both commands (and avoid relying on a prior os.Chdir). This also prevents go.mod being created in the wrong directory if createBasic is ever called without createProject.
| if err := runCmd(execCommand("go", "mod", "init", modName)); err != nil { | |
| return err | |
| } | |
| //Execute go mod tidy in the project directory | |
| installModules := execCommand("go", "mod", "tidy") | |
| installModules.Dir = fmt.Sprintf("%s%c", projectPath, os.PathSeparator) | |
| modInit := execCommand("go", "mod", "init", modName) | |
| modInit.Dir = projectPath | |
| if err := runCmd(modInit); err != nil { | |
| return err | |
| } | |
| // Execute go mod tidy in the project directory | |
| installModules := execCommand("go", "mod", "tidy") | |
| installModules.Dir = projectPath |
| installModules := execCommand("go", "mod", "tidy") | ||
| installModules.Dir = fmt.Sprintf("%s%c", projectPath, os.PathSeparator) | ||
| if err := runCmd(installModules); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
go mod tidy can be affected by an enclosing go.work (workspace) file. Elsewhere the repo explicitly forces GOWORK=off when running go mod ... commands (see cmd/internal/go_mod.go:33-55). Consider applying the same environment filtering here for both go mod init and go mod tidy so project generation is isolated from the caller’s workspace.
| installModules := execCommand("go", "mod", "tidy") | ||
| installModules.Dir = fmt.Sprintf("%s%c", projectPath, os.PathSeparator) | ||
| if err := runCmd(installModules); err != nil { |
There was a problem hiding this comment.
installModules.Dir is set via fmt.Sprintf("%s%c", projectPath, os.PathSeparator), which adds a trailing separator and is inconsistent with other usages (cmd.Dir = dir in cmd/internal/go_mod.go). Prefer setting Dir to projectPath (optionally cleaned with filepath.Clean) to avoid odd paths like .../project// and keep path handling consistent.
| installModules := execCommand("go", "mod", "tidy") | ||
| installModules.Dir = fmt.Sprintf("%s%c", projectPath, os.PathSeparator) | ||
| if err := runCmd(installModules); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
This change adds a new externally-visible step (go mod tidy) during fiber new, but current tests don’t assert that tidy is invoked. Consider adding a unit test that captures execCommand invocations and verifies go mod tidy runs after go mod init.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
1 similar comment
|
@copilot open a new pull request to apply changes based on the comments in this thread |
4 participants
Please provide enough information so that others can review your pull request:
Fix to #122
Explain the details for making this change. What existing problem does the pull request solve?
The expected use case for
fiber new <name_project>is a working and ready to go setup. With this PR i propose to automatically install the required modules usinggo mod tidySummary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.