-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Point to new ko repo #654
Point to new ko repo #654
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ImJasonH, vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@imjasonh i think i horribly broke prow, hold plz 😇 |
It turns out that calling `t.Fatal` (and other functions that end tests, see https://tip.golang.org/pkg/testing/#T) is only safe from the goroutine running the Test function, so when this test started to fail (root cause still TBD) (e.g. in tektoncd#654) we started to see race condition failures, kinda masking whatever was actually going wrong. I think I caught all the instances of this but not 100% sure - note that using it in sub tests (https://blog.golang.org/subtests) is a-ok :D
Turns out it was slightly more complicated: #655 And actually we should hold off on merging this in until https://github.com/google/ko is brought up to date (specifically to include google/go-containerregistry#380) /hold |
It turns out that calling `t.Fatal` (and other functions that end tests, see https://tip.golang.org/pkg/testing/#T) is only safe from the goroutine running the Test function, so when this test started to fail (root cause still TBD) (e.g. in #654) we started to see race condition failures, kinda masking whatever was actually going wrong. I think I caught all the instances of this but not 100% sure - note that using it in sub tests (https://blog.golang.org/subtests) is a-ok :D
@imjasonh this is fixed now in the ko repo right 😇 /lgtm |
@bobcatfish: Bad category. Please see https://api.thecatapi.com/api/categories/list In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
🤔 /meow space |
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/hold cancel |
/test pull-tekton-pipeline-integration-tests |
Changes
ko
has moved to https://github.com/google/koSubmitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide
for more details.
Release Notes