-
Notifications
You must be signed in to change notification settings - Fork 132
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: add function to create a more speaking id for commit status #705
base: main
Are you sure you want to change the base?
feat: add function to create a more speaking id for commit status #705
Conversation
ecbcebf
to
9a7daac
Compare
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 change needs to be accompanied by documentation. I'm also not sure how much we're bought into the current format as people might already be relying on it as it is right now. @stefanprodan?
@@ -59,10 +59,29 @@ func formatNameAndDescription(event eventv1.Event) (string, string) { | |||
// involved object kind and name. | |||
func generateCommitStatusID(providerUID string, event eventv1.Event) string { | |||
uidParts := strings.Split(providerUID, "-") | |||
id := fmt.Sprintf("%s/%s/%s", event.InvolvedObject.Kind, event.InvolvedObject.Name, uidParts[0]) | |||
metadataLabelsSuffix := createMetadataLabelsSuffix(event) |
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 has the potential to result in a very long string which is undesirable. We need to think on a way to constrain this to a certain length.
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.
true, it might be an option to change eventMetadata keys accepted for building an extended commit status ID? @makkes
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.
any suggestion how to proceed @makkes @stefanprodan ?
Signed-off-by: Brandt, Sebastian (ITO-PC) <[email protected]>
9a7daac
to
fa99576
Compare
@@ -1474,6 +1474,34 @@ spec: | |||
name: flux-system | |||
``` | |||
|
|||
##### Construction of Git Commit Status ID |
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 should explain here what the commit status ID actually is and how it can be leveraged.
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.
Is it might be enough to link to GitHub and GitLab related API endpoints - as this documentation stays up2date? @makkes
GitHub: https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28
GitLab: https://docs.gitlab.com/ee/api/commits.html#set-the-pipeline-status-of-a-commit
Co-authored-by: Max Jonas Werner <[email protected]> Signed-off-by: Sebastian Brandt <[email protected]>
Co-authored-by: Max Jonas Werner <[email protected]> Signed-off-by: Sebastian Brandt <[email protected]>
Add a function to get a more detailed insight into the commit status of e.g. GitLab.
So that you do not need to lookup the first part of the Providers UIDs at all your Flux powered Kubernetes clusters.