-
Notifications
You must be signed in to change notification settings - Fork 209
Make pipectl plan-preview command renders the results to stdout and file #2154
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
Conversation
|
The following files are not gofmt-ed. By commenting pkg/app/pipectl/cmd/planpreview/planpreview_test.go--- pkg/app/pipectl/cmd/planpreview/planpreview_test.go.orig
+++ pkg/app/pipectl/cmd/planpreview/planpreview_test.go
@@ -39,12 +39,12 @@
{
name: "there is only a plannable application",
results: []*model.PlanPreviewCommandResult{
- &model.PlanPreviewCommandResult{
+ {
CommandId: "command-2",
PipedId: "piped-2",
PipedUrl: "https://pipecd.dev/piped-2",
Results: []*model.ApplicationPlanPreviewResult{
- &model.ApplicationPlanPreviewResult{
+ {
ApplicationId: "app-1",
ApplicationName: "app-1",
ApplicationUrl: "https://pipecd.dev/app-1",
@@ -67,12 +67,12 @@
{
name: "there is only a failure application",
results: []*model.PlanPreviewCommandResult{
- &model.PlanPreviewCommandResult{
+ {
CommandId: "command-2",
PipedId: "piped-2",
PipedUrl: "https://pipecd.dev/piped-2",
Results: []*model.ApplicationPlanPreviewResult{
- &model.ApplicationPlanPreviewResult{
+ {
ApplicationId: "app-2",
ApplicationName: "app-2",
ApplicationUrl: "https://pipecd.dev/app-2",
@@ -93,7 +93,7 @@
{
name: "there is only a failure piped",
results: []*model.PlanPreviewCommandResult{
- &model.PlanPreviewCommandResult{
+ {
CommandId: "command-1",
PipedId: "piped-1",
PipedUrl: "https://pipecd.dev/piped-1",
@@ -110,18 +110,18 @@
{
name: "all kinds",
results: []*model.PlanPreviewCommandResult{
- &model.PlanPreviewCommandResult{
+ {
CommandId: "command-1",
PipedId: "piped-1",
PipedUrl: "https://pipecd.dev/piped-1",
Error: "failed to clone",
},
- &model.PlanPreviewCommandResult{
+ {
CommandId: "command-2",
PipedId: "piped-2",
PipedUrl: "https://pipecd.dev/piped-2",
Results: []*model.ApplicationPlanPreviewResult{
- &model.ApplicationPlanPreviewResult{
+ {
ApplicationId: "app-1",
ApplicationName: "app-1",
ApplicationUrl: "https://pipecd.dev/app-1",
@@ -130,7 +130,7 @@
SyncStrategy: model.SyncStrategy_QUICK_SYNC,
Changes: []byte("changes-1"),
},
- &model.ApplicationPlanPreviewResult{
+ {
ApplicationId: "app-2",
ApplicationName: "app-2",
ApplicationUrl: "https://pipecd.dev/app-2",
@@ -139,7 +139,7 @@
SyncStrategy: model.SyncStrategy_PIPELINE,
Changes: []byte("changes-2"),
},
- &model.ApplicationPlanPreviewResult{
+ {
ApplicationId: "app-3",
ApplicationName: "app-3",
ApplicationUrl: "https://pipecd.dev/app-3",
@@ -147,7 +147,7 @@
EnvName: "env-3",
Error: "wrong application configuration",
},
- &model.ApplicationPlanPreviewResult{
+ {
ApplicationId: "app-4",
ApplicationName: "app-4",
ApplicationUrl: "https://pipecd.dev/app-4",
@@ -157,7 +157,7 @@
},
},
},
- &model.PlanPreviewCommandResult{
+ {
CommandId: "command-3",
PipedId: "piped-3",
PipedUrl: "https://pipecd.dev/piped-3",
|
pipecd-bot
left a comment
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.
|
/golinter fmt |
|
Code coverage for golang is
|
|
Code coverage for golang is
|
| // All URL fields inside the result model are empty. | ||
| // So we fill them before sending to the client. | ||
| for _, r := range results { | ||
| r.FillURLs(a.webBaseURL) |
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.
nit: I thought putting this inside right after the loop is enough (like you did before commit 😄 ). But if you intend to be more clear, it's on you
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.
I will keep this as it is because there are 2 loops (L480, L512) that are updating this list.
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.
That is true 👍
|
The golinter build is completed with FAILURE. The build will be triggered again when you push any other commits. Or you can trigger it manually by You can check the build log from here. |
|
Code coverage for golang is
|
|
excellent! |
|
Way to go 🚀 |
What this PR does / why we need it:
This PR updates
pipectlto make itsplan-previewcommand renders the results to both stdout and a given file.This also adds the needed fields to the PlanPreview's models.
Which issue(s) this PR fixes:
Fixes #2109
Does this PR introduce a user-facing change?: