-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
feat(cmd/init): change default module name to project name #3303
Conversation
WalkthroughThe recent changes in the Wails project enhance the functionality to automatically set the Go module name in the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (5)
- v2/cmd/wails/flags/init.go (1 hunks)
- v2/cmd/wails/init.go (2 hunks)
- v2/cmd/wails/internal/gomod/gomod.go (1 hunks)
- v2/internal/gomod/gomod.go (1 hunks)
- v2/internal/gomod/gomod_test.go (1 hunks)
Additional comments: 4
v2/cmd/wails/flags/init.go (1)
- 14-14: The addition of the
InitModule
field to theInit
struct is correctly implemented, with appropriate tags for CLI parsing and a clear description. Ensure that the CLI documentation is updated to include the new-m
parameter.v2/internal/gomod/gomod.go (1)
- 116-124: The
UpdateGoModuleName
function is implemented correctly, efficiently updating the module name in thego.mod
file. It's worth considering future enhancements to handle edge cases, such asgo.mod
files without a module directive, though such cases are rare in practice.v2/internal/gomod/gomod_test.go (1)
- 155-180: The
TestUpdateGoModName
function correctly tests the functionality of updating the module name in thego.mod
file. Consider adding more test cases to cover edge cases or unusual module names to ensure robustness.v2/cmd/wails/init.go (1)
- 129-135: The integration of changing the module name based on the
InitModule
flag within theinitProject
function is implemented correctly. Ensure this functionality is thoroughly tested across different environments to confirm its reliability.
v2/cmd/wails/internal/gomod/gomod.go
Outdated
func ChangeModuleName(newModuleName string) error { | ||
cwd, err := os.Getwd() | ||
if err != nil { | ||
return err | ||
} | ||
gomodFilename := fs.FindFileInParents(cwd, "go.mod") | ||
if gomodFilename == "" { | ||
return fmt.Errorf("no go.mod file found") | ||
} | ||
gomodData, err := os.ReadFile(gomodFilename) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
gomodData, err = gomod.UpdateGoModuleName(gomodData, newModuleName) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
return os.WriteFile(gomodFilename, gomodData, 0o755) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ChangeModuleName
function is well-structured and follows a logical flow for updating the module name in the go.mod
file. However, the file permissions for the go.mod
file are set to 0o755
when writing back the changes. Typically, go.mod
is a text file and should have permissions set to 0o644
to reflect that it's not an executable file.
- return os.WriteFile(gomodFilename, gomodData, 0o755)
+ return os.WriteFile(gomodFilename, gomodData, 0o644)
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
func ChangeModuleName(newModuleName string) error { | |
cwd, err := os.Getwd() | |
if err != nil { | |
return err | |
} | |
gomodFilename := fs.FindFileInParents(cwd, "go.mod") | |
if gomodFilename == "" { | |
return fmt.Errorf("no go.mod file found") | |
} | |
gomodData, err := os.ReadFile(gomodFilename) | |
if err != nil { | |
return err | |
} | |
gomodData, err = gomod.UpdateGoModuleName(gomodData, newModuleName) | |
if err != nil { | |
return err | |
} | |
return os.WriteFile(gomodFilename, gomodData, 0o755) | |
func ChangeModuleName(newModuleName string) error { | |
cwd, err := os.Getwd() | |
if err != nil { | |
return err | |
} | |
gomodFilename := fs.FindFileInParents(cwd, "go.mod") | |
if gomodFilename == "" { | |
return fmt.Errorf("no go.mod file found") | |
} | |
gomodData, err := os.ReadFile(gomodFilename) | |
if err != nil { | |
return err | |
} | |
gomodData, err = gomod.UpdateGoModuleName(gomodData, newModuleName) | |
if err != nil { | |
return err | |
} | |
return os.WriteFile(gomodFilename, gomodData, 0o644) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- v2/cmd/wails/internal/gomod/gomod.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- v2/cmd/wails/internal/gomod/gomod.go
v2/internal/gomod/gomod.go
Outdated
@@ -112,3 +112,13 @@ func SyncGoVersion(goModText []byte, goVersion string) ([]byte, bool, error) { | |||
|
|||
return goModText, true, nil | |||
} | |||
|
|||
func UpdateGoModuleName(goModText []byte, newModuleName string) ([]byte, error) { | |||
file, err := modfile.Parse("", goModText, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No doubt this works just fine, but I'm wondering if go mod edit -module <name>
would be more forward compatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! This is indeed a lack of consideration on my part. Compared with calling the golang.org/x/mod/modfile
library to parse go.mod
, the latter is more forward-compatible using commands. I will update its implementation and update the PR.
This is a great start! |
of course I'd love to do that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (3)
- v2/cmd/wails/init.go (1 hunks)
- website/docs/reference/cli.mdx (1 hunks)
- website/src/pages/changelog.mdx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- v2/cmd/wails/init.go
Additional comments: 1
website/src/pages/changelog.mdx (1)
- 30-30: The changelog entry for the new
initModule
command flag is clear, concise, and follows the established format. It correctly identifies the type of change, provides a brief description, and credits the contributor.
I would also recommend that the default name of the module be set to the name of the directory created for the project if not overridden with the i.e. use |
Thanks for your suggestion, whenever I create a project, I manually modify |
@leaanthony If I remove the |
Sure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- v2/cmd/wails/init.go (2 hunks)
- website/src/pages/changelog.mdx (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- v2/cmd/wails/init.go
- website/src/pages/changelog.mdx
@leaanthony Ready for review. |
Description
Change the default module name
changeme
to the project name.Fixes #3277
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor
.Test Configuration
Wails
Version | v2.8.0
Package Manager | pacman
System
Dependencies
Checklist:
website/src/pages/changelog.mdx
with details of this PRSummary by CodeRabbit
go.mod
file based on user input.-m
to change thego.mod
module name to the project name.