Add new command fore create migration file#4
Open
Rototot wants to merge 1 commit intogozix:masterfrom
Open
Conversation
Rototot
force-pushed
the
add-new-create-command
branch
from
28a5bc8 to
d9b9105
Compare
snovichkov
reviewed
Aug 1, 2023
internal/command/migrate_create.go
Outdated
| } | ||
|
|
||
| func create(migrationDir string, name string, extension string) (string, error) { | ||
| fileName := filepath.Join( |
Member
There was a problem hiding this comment.
Merge please all variables declaration to var block.
internal/command/migrate_create.go
Outdated
| "", | ||
| }, "\n") | ||
|
|
||
| f, err := os.OpenFile(fileName, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) |
Member
There was a problem hiding this comment.
Same code style notice use var instead of :=.
internal/command/migrate_create.go
Outdated
| if err != nil { | ||
| return fileName, err | ||
| } | ||
| defer func(f *os.File) { |
Member
There was a problem hiding this comment.
Remove please defer function argument because copy pointers this is bad idea.
internal/command/migrate_create.go
Outdated
| }(f) | ||
|
|
||
| _, err = f.WriteString(fileContent) | ||
| if err != nil { |
Member
There was a problem hiding this comment.
Simplify this to return fileName, err
internal/command/migrate_create.go
Outdated
| Args: cobra.MaximumNArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| return ctn.Call(func(ctx context.Context, logger *zap.Logger) (err error) { | ||
| const extension string = "sql" |
Member
There was a problem hiding this comment.
Make please this const as package global const.
internal/command/migrate_create.go
Outdated
| However some SQL commands (for example creating an index concurrently in PostgreSQL) | ||
| cannot be executed inside a transaction. In order to execute such a command in a migration, | ||
| the migration can be run using the notransaction option https://github.com/rubenv/sql-migrate`, | ||
| Args: cobra.MaximumNArgs(1), |
Member
There was a problem hiding this comment.
Use cobra.ExactArgs(1) instead of cobra.MaximumNArgs(1)
internal/command/migrate_create.go
Outdated
| return ctn.Call(func(ctx context.Context, logger *zap.Logger) (err error) { | ||
| const extension string = "sql" | ||
|
|
||
| var name = "example" |
Member
There was a problem hiding this comment.
This block should be removed.
Rototot
force-pushed
the
add-new-create-command
branch
2 times, most recently
from
19a62c5 to
d82b676
Compare
Rototot
force-pushed
the
add-new-create-command
branch
from
231bfee to
c447944
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.