Skip to content

chore: Use GH_CQ_BOT token to comment on Benchmark results#436

Merged
hermanschaaf merged 2 commits intomainfrom
use-gh-bot-token-to-comment
Nov 25, 2022
Merged

chore: Use GH_CQ_BOT token to comment on Benchmark results#436
hermanschaaf merged 2 commits intomainfrom
use-gh-bot-token-to-comment

Conversation

@hermanschaaf
Copy link
Copy Markdown
Contributor

@hermanschaaf hermanschaaf commented Nov 25, 2022

We need slightly elevated permissions to make a comment when the PR comes from a fork, and I believe this should do the trick.

Github Actions security when it comes to external PRs is a bit of a minefield, but because we're avoiding the pull_request_target trigger here, and not running any repository code in the delta-action itself, I believe this is safe.

This should fix the issue we're seeing here #430

@cq-bot
Copy link
Copy Markdown
Contributor

cq-bot commented Nov 25, 2022

⏱️ Benchmark results

Comparing with 9882458

  • DefaultConcurrency-2 resources/s: 12,140 ⬆️ 0.38% increase vs. 9882458
  • Glob-2 ns/op: 158.4 ⬆️ 0.38% increase vs. 9882458
  • TablesWithChildrenDefaultConcurrency-2 resources/s: 29,996 ⬆️ 2.21% increase vs. 9882458
  • BufferedScanner-2 ns/op: 9.381 ⬇️ 6.71% decrease vs. 9882458
  • LogReader-2 ns/op: 30.75 ⬆️ 0.03% increase vs. 9882458

run: make benchmark-ci
- name: Delta
uses: hermanschaaf/delta-action@v1.0.7
uses: netlify/delta-action@v4.1.0
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also changed back to the upstream version now that my PR got merged in there

Copy link
Copy Markdown
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

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

LGTM.

@hermanschaaf hermanschaaf merged commit 178c32e into main Nov 25, 2022
@hermanschaaf hermanschaaf deleted the use-gh-bot-token-to-comment branch November 25, 2022 09:48
hermanschaaf added a commit that referenced this pull request Nov 25, 2022
The fix in #436 didn't
work; looks like you can't use the GH bot token from a forked repo PR
either.
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