-
Notifications
You must be signed in to change notification settings - Fork 156
WIP: S3 integration #520
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
WIP: S3 integration #520
Conversation
| // VersionArgs are flags for the `RunVersion` function | ||
| type S3Args struct{} | ||
|
|
||
| // Version is the handler for 'scw version' |
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.
comment on exported function S3 should be of the form "S3 ..."
| minio "github.com/minio/mc/cmd" | ||
| ) | ||
|
|
||
| // VersionArgs are flags for the `RunVersion` function |
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.
comment on exported type S3Args should be of the form "S3Args ..." (with optional leading article)
pkg/commands/s3.go
Outdated
| // Version is the handler for 'scw version' | ||
| func S3(ctx CommandContext, args S3Args) error { | ||
| // remove "s3" from arg list | ||
| os.Args = append(os.Args[:1], os.Args[2:]...) |
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.
Don't think It's a good idea to overwrite os.Args, could not you use args ?
pkg/config/config.go
Outdated
| return "", err | ||
| } | ||
| return filepath.Join(path, ".scwrc"), nil | ||
| return filepath.Join(path, ".scw/config.json"), 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.
Will break the legacy ?
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.
It works when ~/.scw doesn't exist ?
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.
Should probably also split the string into .scw and config.json too, instead of embedding the / in it. filepath.Join() will add the OS appropriate separator.
pkg/config/config.go
Outdated
| // mv file to new Path | ||
| err = os.Rename(oldConfigPath, newConfigPath) | ||
| if err != nil { | ||
| fmt.Errorf("%s\n", err) |
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.
error strings should not be capitalized or end with punctuation or a newline
pkg/config/config.go
Outdated
| if os.IsNotExist(err) { | ||
| return | ||
| } | ||
| fmt.Errorf("%s\n", err) |
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.
error strings should not be capitalized or end with punctuation or a newline
pkg/config/config.go
Outdated
| // Get old config Path | ||
| oldConfigPath, err := GetHomeDir() | ||
| if err != nil { | ||
| fmt.Errorf("%s\n", err) |
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.
error strings should not be capitalized or end with punctuation or a newline
| Version string `json:"version"` | ||
| } | ||
|
|
||
| // migrate config from home to ~/.scw/ |
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.
comment on exported function MigrateConfig should be of the form "MigrateConfig ..."
| minio "github.com/minio/mc/cmd" | ||
| ) | ||
|
|
||
| // VersionArgs are flags for the `RunVersion` function |
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.
comment on exported type S3Args should be of the form "S3Args ..." (with optional leading article)
|
See : #529 |
WIP add s3 command
Integrate minio package to scw
TODO: