-
Notifications
You must be signed in to change notification settings - Fork 266
Actually, vendor everything #59
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
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sixolet 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 |
With this change, it means go modules are no longer technically required to build the client repo. And, we should probably add Adding a lgtm with a hold. I'm ok to merge this now and address the related changes in subsequent PRs, but punting that decision to you with the hold. |
Helps if I actually do the prow commands as well... /lgtm |
How much would this help you get e2e tests up and running if we landed it? |
This PR should solve the e2e flake issues as-is. After it lands I'll kick off tests on the e2e PR a few times to confirm. |
According to golang/go#27348 there is no good way to actually verify the contents of the vendor directory. Do you happen to know if I'm missing something? I added a trivial script to go mod vendor things, and I think that might be the best compromise for now. What do you think? |
Thi seems like a good compromise. I'll poke at how we actually verify vendor in other Knative repos - I think they do some special trick with copying files to a tmp directory, vendoring, and comparing. But that can wait. /lgtm |
/hold cancel |
That's correct: https://github.com/knative/serving/blob/master/hack/verify-codegen.sh |
- Earlier we were only running few tests - This runs the e2e tests present in upstream repository
$0 isn't available when sourcing a script. Plus, update `README.md` to clarify how these files appear in other repos.
We were getting errors when trying to build in CI when a backing repository 502'd, so let's vendor all our deps.