Skip to content

Conversation

ianayl
Copy link
Contributor

@ianayl ianayl commented Oct 9, 2025

This PR:

  • moves benchmarking CI data to benchmark-ci-tests branch instead of intel/llvm-ci-perf-results
  • removes the need for an additional bot user when pushing data (as well as the need to periodically update its tokens)

Test run: https://github.com/intel/llvm/actions/runs/18386952310/job/52388325821 (job failed due to "regression"; issue should be addressed with the merge of #20277

@ianayl ianayl requested a review from a team as a code owner October 9, 2025 20:37
@ianayl ianayl requested a review from a team October 9, 2025 20:37

permissions:
contents: read
contents: write
Copy link
Contributor

Choose a reason for hiding this comment

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

can we set the write permission only at the job step(s) that ened it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me attempt this and I'll get back to you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like permissions have a job-level granularity, I can't just give it to a single step.

Doesn't make much sense to nest an additional write permission for the job given that there is only a single job in the entire workflow, are we cool to keep it like this?

env:
RUNNER_TAG: ${{ inputs.runner }}
GITHUB_TOKEN: ${{ secrets.LLVM_SYCL_BENCHMARK_TOKEN }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we try removing this line? i think GITHUB_TOKEN is automatically passed

results_file="$(git diff HEAD~1 --name-only -- results/ | head -n 1)"
if git push "https://[email protected]/intel/llvm-ci-perf-results.git" "$results_branch"; then
if git push; then
Copy link
Contributor

Choose a reason for hiding this comment

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

dont we need to specify the branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks to be already configured by actions/checkout@v5; I figured it was better to specify the branch once than to specify the branch multiple times.

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