Removing vendor submodule #5014
Conversation
|
Did we decide to commit vendor? |
|
AFAIK, the only blocker I was aware of was someone to do the work. And possibly waiting for API to merge into istio/istio. |
|
some description please? |
|
The work includes describing the process to update vendor in the future, not a one time commit. This is going to be an on-going maintenance, so I assume you want to subscribe for that? :) |
|
@rshriram done. |
The istio team is responsible for any maintenance. |
Codecov Report
@@ Coverage Diff @@
## master #5014 +/- ##
=======================================
+ Coverage 74% 74% +1%
=======================================
Files 306 307 +1
Lines 25966 25746 -220
=======================================
- Hits 18976 18832 -144
+ Misses 6239 6165 -74
+ Partials 751 749 -2
Continue to review full report at Codecov.
|
|
please get @geeknoid 's approval as well. |
|
I agree with Shriram. Our guidance should be that try to commit vendor changes separately from other changes, unless there's some sort of nasty dependency that makes that hard. |
|
If you recall the meeting yesterday we decided that we should first move istio/api into istio/ and then revisit Also that we don't want to mess with 0.8 This being said if you are willing to deal with all future vendor issues for at least 2 months cc @hklai |
|
Context (can you add to your PR description, at least) |
no, typing "git add" and deleting lines isn't "the work", the work is ensuring the outcome makes sense and maintaining it. it took a lot more (too much actually) work to setup and to document the flow than this. but given uninformed people once more override the test&release process and just blindly approve... I'm done dealing with vendor. thanks for your future maintenance of this. |
|
please merge this quickly so I get the benefits of freeing up my time, also please inform all the [vendor-change] PRs as well the open PRs in https://github.com/istio/vendor-istio/pulls I'll mark that repo archived once you're done |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hklai 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 |
|
Raised #5023 for defining the new process |
There current vendor submodule provides a problematic developer workflow, requiring 2 separate PRs: one to the vendor repo and one to istio/istio.
The PR to the vendor repository has to be made on a branch of the vendor repository itself (forks are not supported). This disallows external developers for writing any PR that adds a dependency.
Once the vendor PR adding dependency A has been submitted, the developer effectively has a virtual "lock" on the Istio dependencies. The dep tool automatically prunes unused dependencies, so if another developer attempts to add dependency B before A is actually being used by Istio, dependency A will be removed when B is added.
The time it takes between the submission of the 2 PRs presents a general problem to the developer workflow. This PR removes vendor as a submodule altogether and simply imports the vendor directory into istio/istio. After this is committed, future work will require a single PR to add code that imports a new dependency.