-
Notifications
You must be signed in to change notification settings - Fork 12
Add ConvSun Integration #256
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR integrates the ConvSun tool into the Platform.sh CLI to enable conversion of Platform.sh configuration files to Upsun-compatible format. The integration adds a new project:convert
command that creates Upsun configuration files in a .upsun
folder based on existing Platform.sh configurations.
- Adds ConvSun library dependency and updates Go toolchain version
- Implements a new
project:convert
command with aliasconvert
- Integrates the command into both the root command structure and command listing
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
go.mod | Updates Go version to 1.23.5, toolchain to 1.24.4, and adds ConvSun dependencies |
commands/root.go | Registers the new convert command in the root command structure |
commands/list.go | Adds the convert command to the command listing functionality |
commands/convert_config.go | Implements the new convert command that uses ConvSun API to transform configurations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
func NewConvertConfigCommand() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "project:convert ", | ||
Short: "Locally create and upsun config", |
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 short description has a grammatical error. It should be "Locally create an Upsun config" or "Locally create Upsun config".
Short: "Locally create and upsun config", | |
Short: "Locally create an Upsun config", |
Copilot uses AI. Check for mistakes.
api.Convert(rootProject, upsunFolder) | ||
} | ||
|
||
return 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.
The api.Convert function call result is not handled. If the conversion fails, the error should be returned to provide proper feedback to the user.
return nil | |
err = api.Convert(rootProject, upsunFolder) | |
} | |
return err |
Copilot uses AI. Check for mistakes.
if err == nil { | ||
api.Convert(rootProject, upsunFolder) | ||
} | ||
|
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 function always returns nil even when directory creation fails. The error from os.MkdirAll should be returned when it's not nil.
if err != nil { | |
return err | |
} | |
api.Convert(rootProject, upsunFolder) |
Copilot uses AI. Check for mistakes.
No description provided.