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

test: don't use gh cli in kbs checkout #2107

Conversation

mkulke
Copy link
Collaborator

@mkulke mkulke commented Oct 11, 2024

Since we clone the repo we can use git to resolve a tag/branch to a sha. We probably want to avoid using gh cli in places where it's not required, because it involves setting secrets and makes executing tests more cumbersome. There could also be breakage if people specify a fork as KBS_REPO, since we use the gh cli with a hardcoded repository.

Since we clone the repo we can use git to resolve a tag/branch to a
sha. We probably want to avoid using gh cli in places where it's not
required, because it involves setting secrets and makes executing tests
more cumbersome. There could also be breakage if people specify a fork
as KBS_REPO, since we use the gh cli with a hardcoded repository.

Signed-off-by: Magnus Kulke <magnuskulke@microsoft.com>
@mkulke mkulke requested a review from a team as a code owner October 11, 2024 06:53
Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

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

This makes sense given some of the challenges with gh

Copy link
Member

@wainersm wainersm left a comment

Choose a reason for hiding this comment

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

yes, please. gh is starting to remind me of the yq drama :(

@wainersm wainersm merged commit 3f2fc95 into confidential-containers:main Oct 11, 2024
19 checks passed
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