3 ms·

As someone who has worked on large-scale refactorings at Google they usually do happen as the author describes: 1. Add the new interface (& deprecate the old o

As someone who has worked on large-scale refactorings at Google they usually do happen as the author describes: 1. Add the new interface (& deprecate the old one) 2. Migrate callers over (this can take a long time) 3. Remove the old interface. Even then, you risk breakages because in some cases the deprecation relies on creating a list of approved existing callers, and new callers might be checked in while you're generating that list. (In that case you would ask the new callers to fix-forward by adding themselves to the list.) This three step process has to happen because step 2 takes a long time for widely-used interfaces, and automatic refactorings cannot handle all cases. The only time you can consolidate all three into one commit is if the refactoring is so trivial that automatic tooling can handle every case (in which case, does the cost of code churn & code review really justify the change?) or the number of usages are small enough that a person can manually make all the changes the automatic fixer can't handle before much churn happens.

vlovich123 5y ago
Hmm... I didn't work on large scale refactors but (IIRC - it's been a few years) I definitely had to approve commits that were touching components I had ownership over & that was part of the same commit as many other changes batched. Given that I recall a wiki describing what your responsibility was as the downstream user of the API in these scenarios, it seemed like a common thing to me & I recall there were also automated tools to help do this. Whether or not that's the most common workflow is another story. Works great at the C++/Java API layer or trivial reorganizations, may not work as well when modifying runtime behavior since you have to do the 3 phase commit anyway.
- tylerhou 5y ago
  I think it depends on how large the change is. If the change is small — e.g. we need to slightly modify the signature of some function, then the library maintainer would be responsible as they can easily reason about the change (or automatically generate the change) and there is not much benefit to asking the consumer to generate it. Approving commits is part of any (non-global approval) LSC process; originally I meant that there's a lot more friction in a consumer generating code changes than approving them. If you received an email saying that you had to make a bunch of small changes to your codebase, you would probably ignore it and keep working on your P1/P2; on the other hand if you receive a small, reasonable CL from a library team updating their function usages you would likely LGTM without much thought. On the other hand, if the change is: we need to completely overhaul the interface of this library/service, then you need more in-depth reasoning and the consumer team is responsible for making the change (similar to deprecating a service), since these large "deprecations" require a lot more knowledge of how consumers use the code. There is also a middle ground where the changes are somewhat easy to reason about but still require some human intervention; then the task is usually assigned to volunteers.

vlovich123 5y ago

Hmm... I didn't work on large scale refactors but (IIRC - it's been a few years) I definitely had to approve commits that were touching components I had ownership over & that was part of the same commit as many other changes batched. Given that I recall a wiki describing what your responsibility was as the downstream user of the API in these scenarios, it seemed like a common thing to me & I recall there were also automated tools to help do this. Whether or not that's the most common workflow is another story. Works great at the C++/Java API layer or trivial reorganizations, may not work as well when modifying runtime behavior since you have to do the 3 phase commit anyway.

tylerhou 5y ago
I think it depends on how large the change is. If the change is small — e.g. we need to slightly modify the signature of some function, then the library maintainer would be responsible as they can easily reason about the change (or automatically generate the change) and there is not much benefit to asking the consumer to generate it. Approving commits is part of any (non-global approval) LSC process; originally I meant that there's a lot more friction in a consumer generating code changes than approving them. If you received an email saying that you had to make a bunch of small changes to your codebase, you would probably ignore it and keep working on your P1/P2; on the other hand if you receive a small, reasonable CL from a library team updating their function usages you would likely LGTM without much thought. On the other hand, if the change is: we need to completely overhaul the interface of this library/service, then you need more in-depth reasoning and the consumer team is responsible for making the change (similar to deprecating a service), since these large "deprecations" require a lot more knowledge of how consumers use the code. There is also a middle ground where the changes are somewhat easy to reason about but still require some human intervention; then the task is usually assigned to volunteers.