3 ms·

> On top of the baggage it adds to the repository, it also makes you re-review the same dependencies in multiple projects. Huh? Why would that be the case? (

by cxr 4y ago

> On top of the baggage it adds to the repository, it also makes you re-review the same dependencies in multiple projects. Huh? Why would that be the case? (You're saying storing the code of your dependencies directly in your source tree would require you to reread the code for every project where that dependency is used? Or something else?) > big dumps will get a careless "LGTM" Your "will" means "probably will" (i.e., "can"), but when package registries are involved for late-fetching dependencies, that's what goes on, too, every time. You say this, though, as if it's a unique downside to in-tree dependencies.

pornel 4y ago
Unless you have a company-wide monorepo, you'll end up needing to vendor the same dependencies on separate occasions in multiple projects. If you use vendoring as an opportunity to review code of dependencies you're adding, you end up with multiple PRs that include the same code you've already reviewed. In some cases you may remember you've already reviewed code of a certain dependency and can skip it next time, but human memory is imprecise (have you reviewed this exact version or an older one?), and harder to share between team members to avoid duplicate effort across the team (if someone else gets a vendoring PR, how will they know what you've already reviewed before?) Tendency to "LGTM" grows with code size. If you get a massive PR that vendors fifty deps at once, it will be an intimidating wall of code, under pressure to quickly accept the PR to unblock development. A tool that shares and "recycles" prior reviews may reduce amount of code you review. It lets you review one package at a time, and split that work among team members. Depending on your threat model you may also review deps asynchronously, not blocking PRs.
- cxr 4y ago
  To reiterate: "LGTM" is what happens right now with most out-of-tree dependencies listed in package.json and fetched JIT. To put it another way: keeping your dependencies in-tree or not doesn't directly impact code size. All the code is there in either case, whether you're checking it out as a consequence of cloning the repo or whether you're pulling it down via npm install. You're either already reviewing it, or you're not. On that specific factor you brought up (tendency to "LGTM"), there is no unique downside to storing the dependencies in-tree. It's illusory thinking to believe that there is.

by cxr 4y ago

pornel 4y ago

Unless you have a company-wide monorepo, you'll end up needing to vendor the same dependencies on separate occasions in multiple projects. If you use vendoring as an opportunity to review code of dependencies you're adding, you end up with multiple PRs that include the same code you've already reviewed. In some cases you may remember you've already reviewed code of a certain dependency and can skip it next time, but human memory is imprecise (have you reviewed this exact version or an older one?), and harder to share between team members to avoid duplicate effort across the team (if someone else gets a vendoring PR, how will they know what you've already reviewed before?) Tendency to "LGTM" grows with code size. If you get a massive PR that vendors fifty deps at once, it will be an intimidating wall of code, under pressure to quickly accept the PR to unblock development. A tool that shares and "recycles" prior reviews may reduce amount of code you review. It lets you review one package at a time, and split that work among team members. Depending on your threat model you may also review deps asynchronously, not blocking PRs.

cxr 4y ago
To reiterate: "LGTM" is what happens right now with most out-of-tree dependencies listed in package.json and fetched JIT. To put it another way: keeping your dependencies in-tree or not doesn't directly impact code size. All the code is there in either case, whether you're checking it out as a consequence of cloning the repo or whether you're pulling it down via npm install. You're either already reviewing it, or you're not. On that specific factor you brought up (tendency to "LGTM"), there is no unique downside to storing the dependencies in-tree. It's illusory thinking to believe that there is.