Skip to content
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

Add option to add stats to Job Summary page #263

Merged
merged 10 commits into from
Dec 6, 2024

Conversation

planetmarshall
Copy link
Contributor

Adds a job-summary option that adds the stats to the Job Summary section of a workflow run. Requires CCache 4.10+

Also added some unit tests and a workflow job for same.

Just added cache hits for now but is easily extendable. - looks like this:

Screenshot From 2024-11-22 18-55-13

@hendrikmuhs
Copy link
Owner

Nice!

Do you see a reason why somebody would decide against this? I understand the need for the version check. Given that the version supports it, I would enable it. Or do I miss anything? Is there a usecase for disabling the output?

@planetmarshall
Copy link
Contributor Author

Nice!

Do you see a reason why somebody would decide against this?

No problem. Just being conservative - I'll update the PR enabling the feature by default.

@TrentHouliston
Copy link
Contributor

TrentHouliston commented Nov 25, 2024

I've done something similar for a private repo, Something that I found looks really good is to use a mermaid diagram to show the direct/preprocessed/missed stats in a pie chart diagram.

https://mermaid.js.org/syntax/pie.html

Since the summaries support github flavoured markdown they render right in the summary view!

e.g.

pie showData title CCache Summary
    "Direct Hits" : 149818
    "Preprocessed Hits" : 1840
    "Misses" : 3743
pie showData title CCache Summary
    "Direct Hits" : 149818
    "Preprocessed Hits" : 1840
    "Misses" : 3743
Loading

@hendrikmuhs
Copy link
Owner

@planetmarshall Thanks again for the contribution. I think this ready to be merged, however I meanwhile merged some dependabot PRs and caused a merge conflict.

Shall I fix it myself or can you?
I also wonder, do you want to lock into the suggestion from @TrentHouliston? Or is that something for a follow up? - I definitely like the idea and support it, however it could be done in a separate change.

# Conflicts:
#	package-lock.json
#	package.json
@planetmarshall
Copy link
Contributor Author

@planetmarshall Thanks again for the contribution. I think this ready to be merged, however I meanwhile merged some dependabot PRs and caused a merge conflict.

Shall I fix it myself or can you?

I've updated the branch.

I also wonder, do you want to lock into the suggestion from @TrentHouliston? Or is that something for a follow up? - I definitely like the idea and support it, however it could be done in a separate change.

I like the the idea but I think it's better added as a future enhancement.

@hendrikmuhs hendrikmuhs merged commit 6f38785 into hendrikmuhs:main Dec 6, 2024
42 checks passed
@planetmarshall planetmarshall deleted the stats-job-summary branch December 6, 2024 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants