Change insight proto definition #3403
Merged
Conversation
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
Collaborator
|
The following files are not gofmt-ed. By commenting pkg/app/server/service/webservice/service.pb.go--- pkg/app/server/service/webservice/service.pb.go.orig
+++ pkg/app/server/service/webservice/service.pb.go
@@ -21,13 +21,15 @@
package webservice
import (
+ reflect "reflect"
+ sync "sync"
+
_ "github.com/envoyproxy/protoc-gen-validate/validate"
- model "github.com/pipe-cd/pipecd/pkg/model"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
wrapperspb "google.golang.org/protobuf/types/known/wrapperspb"
- reflect "reflect"
- sync "sync"
+
+ model "github.com/pipe-cd/pipecd/pkg/model"
)
const (
pkg/insight/insightstore/insightstoretest/insightstore.mock.go--- pkg/insight/insightstore/insightstoretest/insightstore.mock.go.orig
+++ pkg/insight/insightstore/insightstoretest/insightstore.mock.go
@@ -9,6 +9,7 @@
reflect "reflect"
gomock "github.com/golang/mock/gomock"
+
insight "github.com/pipe-cd/pipecd/pkg/insight"
)
pkg/model/insight.pb.go--- pkg/model/insight.pb.go.orig
+++ pkg/model/insight.pb.go
@@ -21,11 +21,12 @@
package model
import (
+ reflect "reflect"
+ sync "sync"
+
_ "github.com/envoyproxy/protoc-gen-validate/validate"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
- reflect "reflect"
- sync "sync"
)
const (
|
This comment was marked as outdated.
This comment was marked as outdated.
Member
Author
|
/golinter fmt |
Collaborator
|
Unabled to format the unfmt-ed files. In response to this comment. |
nghialv
reviewed
Mar 17, 2022
| int64 data_point_count = 4 [(validate.rules).int64.gt = 0]; | ||
| string application_id = 5; | ||
| int64 range_from = 2 [(validate.rules).int64.gt = 0]; | ||
| int64 day_count = 3 [(validate.rules).int64.gt = 0]; |
Member
There was a problem hiding this comment.
How about changing this field to range_to to be unified?
Member
Author
There was a problem hiding this comment.
Thank you!
At first I had set it to range_to, but after seeing #1187, I changed it to day_count. This is because dealing time is complicated.
Member
There was a problem hiding this comment.
It was difficult because we had many UNIT types (DAILY, WEEKLY, ...) before.
But now only Daily is supported and I think it could be simple. 🤔
Member
Author
There was a problem hiding this comment.
I got your point. I'll change it to use range_to. Thank you!
This comment was marked as outdated.
This comment was marked as outdated.
knanao
reviewed
Mar 17, 2022
Member
Author
|
I completely forgot about web. |
This comment was marked as outdated.
This comment was marked as outdated.
Collaborator
|
The following files are not gofmt-ed. By commenting pkg/app/server/service/webservice/service.pb.go--- pkg/app/server/service/webservice/service.pb.go.orig
+++ pkg/app/server/service/webservice/service.pb.go
@@ -21,13 +21,15 @@
package webservice
import (
+ reflect "reflect"
+ sync "sync"
+
_ "github.com/envoyproxy/protoc-gen-validate/validate"
- model "github.com/pipe-cd/pipecd/pkg/model"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
wrapperspb "google.golang.org/protobuf/types/known/wrapperspb"
- reflect "reflect"
- sync "sync"
+
+ model "github.com/pipe-cd/pipecd/pkg/model"
)
const (
|
This comment was marked as outdated.
This comment was marked as outdated.
Collaborator
|
The following ISSUES will be created once got merged. If you want me to skip creating the issue, you can use Details1. Enable fetch chart data on insight filter changes.pipecd/web/src/components/insight-page/insight-header/index.tsx Lines 104 to 107 in ff4369d This was created by todo plugin since "TODO:" was found in ff4369d when #3403 was merged. cc: @Hosshii. |
Collaborator
|
Code coverage for golang is
|
Collaborator
|
Code coverage for javascript is
|
Member
Author
|
I fixed web test and proto definition. PTAL |
Member
|
Go ahead. /lgtm |
Member
|
Go bold |
Collaborator
|
This pull request is APPROVED by Approvers can cancel the approval by writing |
This was referenced Mar 17, 2022
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this PR does / why we need it:
In order to run in a production environment, we will implement only the deployment frequency for now. In addition, we have changed our implementation policy a bit, eliminating weekly, monthly, and yearly, and only implementing daily.
This PR is a change to the proto file for this purpose. The current implementation needs to be modified accordingly, but I have commented it out because it requires a significant amount of modification and the policy has changed a bit. I will uncomment the commented out sections as I modify them in future PR's.
Which issue(s) this PR fixes:
ref #1142
Does this PR introduce a user-facing change?: