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

Fix API docs and benchmark dashboards #314

Merged
merged 62 commits into from
Oct 9, 2020

Conversation

echeran
Copy link
Contributor

@echeran echeran commented Oct 9, 2020

Main fixes to get benchmark dashboards to generate correctly and coexist API docs was tested in echeran#10.

There were a couple of problems:

  • somehow, the path of the benchmark output files dropped a path segment compared to what I thought I had made it
  • even though the benchmark action says to use a cache action, that doesn't work for us (I'm confused how that would make sense for anyone, really)
  • we still have to store the commits in a local branch for storing history
  • we should use the upload & download actions (instead of caching) to persist the benchmark dashboard output files across jobs within a workflow, collect them together with the API docs, and copy them at the same time to our docs repo

A second test PR was create to test the split nature behavior for PRs -- we want unmerged PRs to not overwrite our dashboards and API docs, and instead only allow PR merges to master to update those things: echeran#12. As verification:

I have one benchmark dashboard and API docs. All the dashboards behave independently, now, so if one works, they all should work.

I looked at the link from @zbraniecki to the repo using Rust with benchmark dashboards. There were a couple of things that got in the way of adapting their setup for our repo:

  1. they seem to run their benchmarks all at once. By contrast, we want to run our benchmarks on a per-component basis, so that we allow the possibility for meaningful alerts that are based on per-component comparisons, not global change comparisons.
  2. they made their own GH action for doing performance comparisons, but it didn't work for me

echeran added 30 commits October 6, 2020 18:49
…g happen once; temporarily remove guard for main branch only triggers to help testing
…race condition in copying GH pages to docs repo
…prevent race condition in copying GH pages to docs repo"

This reverts commit 5ee1e54.
@echeran echeran requested a review from a team as a code owner October 9, 2020 07:50
@echeran echeran requested review from filmil, sffc and zbraniecki October 9, 2020 07:50
@echeran echeran linked an issue Oct 9, 2020 that may be closed by this pull request
@echeran
Copy link
Contributor Author

echeran commented Oct 9, 2020

Because we want to keep our canonical historical benchmark data (output from benchmark runs on commits in master) separate from other benchmark data (output from benchmark runs on unfinished/WIP PRs), the workflow uses the gh-pages and unmerged-pr-bench-data branches to store those 2 categories of benchmark data. It requires a one-time creation of those branches in the repo.

I forgot to create the latter branch in our upstream repo, but I just now did that using these commands and re-started the workflow:

git checkout --orphan unmerged-pr-bench-data
git commit --allow-empty -m "root commit"
git push upstream unmerged-pr-bench-data:unmerged-pr-bench-data

@filmil
Copy link
Contributor

filmil commented Oct 9, 2020 via email

Copy link
Member

@zbraniecki zbraniecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@sffc sffc removed their request for review October 9, 2020 17:45
@sffc sffc merged commit 30ca95f into unicode-org:master Oct 9, 2020
@echeran echeran deleted the api-docs-ci-dash branch June 17, 2021 03:03
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.

Fix docs repo for API docs and dashboards
4 participants