-
Notifications
You must be signed in to change notification settings - Fork 331
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
[main] Upgrade to latest dependencies #2810
[main] Upgrade to latest dependencies #2810
Conversation
c93c618
to
d1793b8
Compare
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #2810 +/- ##
==========================================
- Coverage 81.84% 81.78% -0.06%
==========================================
Files 167 167
Lines 10217 10217
==========================================
- Hits 8362 8356 -6
- Misses 1610 1614 +4
- Partials 245 247 +2 ☔ View full report in Codecov by Sentry. |
eval "$orig_pipefail_opt" | ||
|
||
if ! [ -d vendor ]; then | ||
if ! [[ "${FORCE_VENDOR:-false}" == "true" ]] && ! [ -d vendor ]; then |
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.
@cardil rather than an env var why don't we just infer repos want to use vendor if they already have a vendor
folder?
This would mean things should just work as expected until repos remove vendor themselves?
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.
Another question are we skipping update_licenses
call if there is no vendor? If so why?
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.
It already works like you're suggesting.
If you don't have a vendor dir, it will not be created for you. If you have one, it will be updated as it was before. You could set this environment flag to force the creation of vendor dir, which I imagine being handy for Knative's vendors.
The license saving is removed here, yes, if there's no vendor dir. But, remember we still do the license scan during the --build-test
run. I believe that satisfies the CNCF license guidelines AFAIK.
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.
The license saving is removed here, yes, if there's no vendor dir. But, remember we still do the license scan during the --build-test run. I believe that satisfies the CNCF license guidelines AFAIK.
We ship licenses in our containers - we shouldn't stop doing that unless we get clear guidance from the CNCF that we no longer need to do that
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.
So I would expect we still run even if we don't have a vendor folder
update_licenses third_party/VENDOR-LICENSE "./..."
526dfb4
to
88e6809
Compare
bumping knative.dev/hack 9cc05a3...0bb79ff: > 0bb79ff Update community files (# 314) > 3af329f Update community files (# 313) > 760813a Allow to not vendor the dependencies (# 311) > f63d16e 🎁 Source embedded hack scripts (# 222) Signed-off-by: Knative Automation <[email protected]>
88e6809
to
5e2a8f1
Compare
/hold /assign @cardil to answer questions |
Unholding - created a separate issue knative/hack#315 |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dprotaso, knative-automation 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 |
Cron -knative-prow-robot
/cc knative/serving-writers knative/eventing-writers
/assign knative/serving-writers knative/eventing-writers
Produced by: knative-extensions/knobots/actions/update-deps