-
Notifications
You must be signed in to change notification settings - Fork 505
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
feat: allow deployment webhook to push deployments to multiple repos … #6535
Conversation
if err != nil { | ||
logger.Error(err, "create deployment commit") | ||
return nil, err | ||
duration := request.FinishedDate.Sub(*request.StartedDate).Seconds() |
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.
duration
is float type now. So please use .Milliseconds()
, and divide 1000, get the float64 duration.
if request.DeploymentCommits == nil { | ||
name = fmt.Sprintf(`deployment for %s`, request.CommitSha) | ||
} else { | ||
commit_sha_list := []string{} |
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.
why not name it with commitShaList
?
} else { | ||
commit_sha_list := []string{} | ||
for _, commit := range request.DeploymentCommits { | ||
commit_sha_list = append(commit_sha_list, commit.CommitSha) |
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.
same as above.
} | ||
} else { | ||
for _, commit := range request.DeploymentCommits { | ||
urlHash16 := fmt.Sprintf("%x", md5.Sum([]byte(commit.RepoUrl)))[:16] |
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 think it is duplicated code, and can be wrapped to a function.
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.
no, this is in the loop
// start_time and end_time is more readable for users, | ||
// StartedDate and FinishedDate is same as columns in db. | ||
// So they all keep. | ||
CreatedDate *time.Time `mapstructure:"create_time"` | ||
StartedDate *time.Time `mapstructure:"start_time" validate:"required"` | ||
FinishedDate *time.Time `mapstructure:"end_time"` | ||
RepoUrl string `mapstructure:"repo_url"` | ||
Environment string `validate:"omitempty,oneof=PRODUCTION STAGING TESTING DEVELOPMENT"` |
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.
why doesn't this field have mapstructure
?
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.
This will not change the name, it will be resolved into environment
all fixed by other pr |
Summary
feat: allow deployment webhook to push deployments to multiple repos in one request
[BE] Support the new payload
[BE] Add commit_msg to table cicd_deployment_commits
[FE] Update webhook deployment example
Does this close any open issues?
Related to #6262
Screenshots
Other Information
Any other information that is important to this PR.