feat: add cli plan command #219
Conversation
| OperationUpdate | ||
| ) | ||
|
|
||
| func (o Operation) String() string { |
There was a problem hiding this comment.
we will handling migrate using plan.Compositor to merge migrate operation (change namespace and update on apply command) between create and delete operation
client/cmd/job/plan.go
Outdated
| } | ||
|
|
||
| jobPlan = &plan.Plan{ProjectName: p.clientConfig.Project.Name, NamespaceName: namespaceName, Kind: plan.KindJob} | ||
| if len(sourceSpec.Name) == 0 && len(destinationSpec.Name) > 0 { |
There was a problem hiding this comment.
want to understand this logic better
There was a problem hiding this comment.
updated the implementation using method as logic description
deryrahman
force-pushed
the
feat-diff-plan
branch
from
98a9d3c
to
21dd699
Compare
okysetiawan
force-pushed
the
feat-diff-plan
branch
from
1f06ed5
to
e89ac4a
Compare
| } | ||
|
|
||
| func (p *planCommand) saveFile(plans plan.Plans) error { | ||
| file, err := os.OpenFile(p.output, unix.O_RDWR|unix.O_CREAT, os.ModePerm) |
There was a problem hiding this comment.
windows environment might not understand this unix package, can we use os.O_RDWR|os.O_CREATE instead?
There was a problem hiding this comment.
we can also use TRUNC mode to avoid manual file truncate. we could use os.O_TRUNC
| directories := providermodel.Diffs(diffs).GetAllDirectories(p.appendDirectory) | ||
| p.logger.Info("job plan found changed in directories: %+v", directories) | ||
|
|
||
| var plans plan.Plans |
There was a problem hiding this comment.
you can encapsulate creation of plans from directories into a single function
| return p.saveFile(mergedPlans) | ||
| } | ||
|
|
||
| func (p *planCommand) describePlanFromDirectory(ctx context.Context, directory string) (*plan.Plan, error) { |
| return nil | ||
| } | ||
|
|
||
| func (p *planCommand) RunE(_ *cobra.Command, _ []string) error { |
There was a problem hiding this comment.
As discussed imo, plan command should not have the logic of plan, it should just invoke the functionality implemented in the plan struct.
There was a problem hiding this comment.
and see if we can create smaller functions & tie them together
| return p.saveFile(mergedPlans) | ||
| } | ||
|
|
||
| func (p *planCommand) describePlanFromDirectory(ctx context.Context, directory string) (*plan.Plan, error) { |
There was a problem hiding this comment.
see if we break it down to smaller functions, the for loops & if else conditions normally we can look into keeping into smaller private functions & we can avoid if else chaining
| return len(sourceSpec.Name) > 0 && len(destinationSpec.Name) == 0 | ||
| } | ||
|
|
||
| func (*planCommand) appendDirectory(directory string, directoryExists map[string]bool, fileDirectories []string) []string { |
There was a problem hiding this comment.
what is the purpose of this appendDirectory can we have this function with a business meaning
| @@ -0,0 +1,26 @@ | |||
| package model | |||
|
|
|||
| type Diff struct { | |||
There was a problem hiding this comment.
what is the purpose of this struct
|
closing this PR, moving to #228 |
Description
Support Git Integration