-
Notifications
You must be signed in to change notification settings - Fork 170
[SDK] Implement ListStageCommands() in SDK #5639
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
Signed-off-by: t-kikuc <[email protected]>
Signed-off-by: t-kikuc <[email protected]>
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.
LNeatTM 🔥
We'll update the version of Go in another PR soon before merging the PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5639 +/- ##
==========================================
- Coverage 26.68% 25.43% -1.26%
==========================================
Files 477 477
Lines 50777 50844 +67
==========================================
- Hits 13552 12931 -621
- Misses 36156 36908 +752
+ Partials 1069 1005 -64 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Thank you!
I commented on comments and namings.
@@ -688,3 +688,32 @@ func (o StageStatus) toModelEnum() model.StageStatus { | |||
return model.StageStatus_STAGE_FAILURE | |||
} | |||
} | |||
|
|||
type StageCommand struct { |
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.
Could you please add comments as others?
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.
Thank you, I fixed it on e95e38b
pkg/plugin/sdk/deployment.go
Outdated
const ( | ||
CommandType_APPROVE_STAGE CommandType = iota | ||
CommandType_SKIP_STAGE | ||
) |
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.
We use PascalCase for naming.
const ( | |
CommandType_APPROVE_STAGE CommandType = iota | |
CommandType_SKIP_STAGE | |
) | |
const ( | |
CommandTypeApproveStage CommandType = iota | |
CommandTypeSkipStage | |
) |
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.
Thank you, I fixed it on e95e38b
Signed-off-by: t-kikuc <[email protected]>
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.
LGTM
@khanhtc1202 Sorry, please approve again 🙏 I fixed nits |
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.
👌
What this PR does:
as title
Why we need it:
Enable handling commands without polling by a plugin developer
Which issue(s) this PR fixes:
Part of #5367
Example Usage
Approval:
Skip:
(cf.
pipecd/pkg/app/piped/executor/analysis/analysis.go
Lines 103 to 126 in 0a5f895
Does this PR introduce a user-facing change?: