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

adding benchmark execution time github action #478

Merged
merged 4 commits into from
Nov 17, 2021

Conversation

tommady
Copy link
Collaborator

@tommady tommady commented Nov 15, 2021

part of #464
this PR is the first part of that issue.

how to use it:

  1. label a PR with benchmark-exec-time
  2. then every push in this PR will run a benchmark action

image

@codecov-commenter
Copy link

codecov-commenter commented Nov 15, 2021

Codecov Report

Merging #478 (06d537d) into main (ba08bb3) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #478   +/-   ##
=======================================
  Coverage   60.44%   60.44%           
=======================================
  Files          82       82           
  Lines       12150    12150           
=======================================
  Hits         7344     7344           
  Misses       4806     4806           

@tommady tommady marked this pull request as draft November 15, 2021 13:28
@tommady tommady marked this pull request as draft November 15, 2021 13:28
uses: marocchino/sticky-pull-request-comment@v2
with:
append: true
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this is not enough permissions in the organization, can you change it to print in actions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK,
I'll figure out a way to print the resulting beauty.

Copy link
Member

@utam0k utam0k Nov 18, 2021

Choose a reason for hiding this comment

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

@saschagrunert Hi! The youki team is trying to measure the results of a very simple benchmark (just for reference) using github actions. I'd like to comment on the results to the PR, but I don't seem to have permission. Could you ask the organizer to check it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rhatdan can we make @utam0k admin of this repository to add secrets for using GitHub actions?

Copy link
Member

@utam0k utam0k Nov 19, 2021

Choose a reason for hiding this comment

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

Thanks for your help.
I am already the owner of this repository.
It looks like there is no write on GITHUB_TOKEN permissions. Isn't GITHUB_TOKEN set to read-only on the organizer side? I could not check the permissions of the organizer with my permissions. In the repository, GITHUB_TOKEN is set to write, but it may be possible to override this permission in the organizer.
https://github.com/containers/youki/pull/478/files#diff-28fe3108ee857a8865d400d072ffeab3d06d0c14073313bd0b39cf4fa41a57fdR134-R142

ref.
https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token

@tommady
Copy link
Collaborator Author

tommady commented Nov 16, 2021

Hi, so I change to print in the terminal, the result will like below in the action

image

@tommady tommady marked this pull request as ready for review November 16, 2021 06:29
Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

This is a great first step.

@utam0k utam0k merged commit ef8770d into youki-dev:main Nov 17, 2021
@tommady tommady deleted the adding-CI-benchmark-github-action-#464 branch November 17, 2021 05:04
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.

4 participants