-
Notifications
You must be signed in to change notification settings - Fork 44
re-write CI analysis command, separate steps/jobs, tests #1669
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
6e621b9 to
0dc94c9
Compare
0dc94c9 to
a692472
Compare
a692472 to
04a730d
Compare
f9e5e93 to
4a468ec
Compare
4a468ec to
7bb8f39
Compare
7bb8f39 to
da6e88d
Compare
da6e88d to
4c78b32
Compare
|
| // analyze jobs | ||
| for _, j := range jobs.Jobs { | ||
| stats.Mu.Lock() | ||
| defer stats.Mu.Unlock() |
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.
isn't it a bad practice to call defer inside for loop?
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.
It leaks memory, true, however, this loop is not inside any service and runtime is very limited, so IMO that's fine.
| } else { | ||
| stats.Jobs[name].Successes++ | ||
| } | ||
| if j.Conclusion != nil && *j.Conclusion == "cancelled" { |
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: define these status strings as constants?
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.
Yep, lemme check if GHA is exposing them
| name = name[:MaxNameLen] | ||
| } | ||
| switch typ { | ||
| case "jobs": |
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: define constants?
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.
Ok, let's do this next time we'll be iterating on it





Below is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.
Why
The changes introduce enhancements and debugging features to the CI analysis tool, including new visualizations, detailed job and step analysis, and debugging capabilities. It also updates dependencies and adds a new testing utility for CI.
What
The enhancements aim to provide users with more detailed insights into their CI processes, focusing on efficiency and reliability. Debugging capabilities allow for a deeper examination of CI workflows, jobs, and steps, facilitating optimization and troubleshooting.