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

summary-always updates benchmarks twice, fails with gh-pages #158

Closed
Kha opened this issue Mar 9, 2023 · 6 comments
Closed

summary-always updates benchmarks twice, fails with gh-pages #158

Kha opened this issue Mar 9, 2023 · 6 comments

Comments

@Kha
Copy link

Kha commented Mar 9, 2023

If summary-always is true, we call first writeBenchmark, then writeSummary.

await writeBenchmark(bench, config);
if (config.summaryAlways) {
await writeSummary(bench, config);
}

But both of these call writeBenchmarkToGitHubPages by default, so we will create the commit twice. In fact we don't even get that far because the second invocation's git fetch will fail to update the gh-pages branch because of the new commit available only locally when auto-push is false.

Example run: https://github.com/Kha/nale/actions/runs/4375198974/jobs/7656891394

@Kha Kha changed the title comment-always does not work without external-data-json-path summary-always updates benchmarks twice, fails with gh-pages Mar 9, 2023
@embano1
Copy link

embano1 commented Mar 12, 2023

Another issue is with the rendering, see: https://github.com/timbray/quamina/actions/runs/4393672754 (example) where the numbers are markdown formatted but not properly rendered.

@Kha
Copy link
Author

Kha commented Mar 12, 2023

@embano1 You should probably open a separate issue for that

@embano1
Copy link

embano1 commented Mar 12, 2023

Yeah, was too lazy as yours was top of list when I just checked...sorry for polluting your issue. #159

@ClementWalter
Copy link

has anyone started to work on this?

abensonca added a commit to galacticusorg/galacticus that referenced this issue Sep 7, 2023
@ktrz
Copy link
Member

ktrz commented Jan 25, 2024

I believe this was fixed by #214
Will verify that tomorrow

@ktrz
Copy link
Member

ktrz commented Jan 26, 2024

Verified!
It is fixed on v1.19.0

@ktrz ktrz closed this as completed Jan 26, 2024
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

No branches or pull requests

4 participants