On Tue, Oct 25, 2016 at 2:33 PM, Kenneth Knowles <[email protected]> wrote: > Hi all, > > While collaborating on the apex-runner branch, the issue of how best to > continuously merge master into the feature branch came up. IMO it differs > somewhat from normal commits in two notable ways: > > 1. Modulo fix-ups, it is actually not adding any new code to the overall > codebase, so reviews don't serve to raise the quality bar for contributions. > 2. It is pretty important to do very frequently to keep out of trouble, so > friction must be thoroughly justified. > > I really wouldn't want reviewing a daily merge from master to consume > someone's time every day. On the gearpump-runner branch we had some major > review latency problems. Obviously, I'd like to hear from folks on other > feature branches. How has this process been for the Python SDK?
The Python SDK sits at the extreme end of there being no conflicts, and due to the paucity of intersection, little motivation to bother merging at all. > I will also throw out an idea for a variation in process: > > 1. A committer merges master to their feature branch without conflicts.* > 2. They open a PR for the merge. > 3a. If the tests pass without modifications _the committer self-merges the > PR without waiting for review_. > 3b. If there are any adjustments needed, these go in separate commits on > the same PR and the review process is as usual (the reviewer can review > just the added commits). > > What do you think? Does this treat such merges too blithely? This is meant > just as a starting point for discussion. > > Kenn > > * There should never be real conflicts unless something strange has > happened - the feature can't be edited on the master branch, and master > stuff shouldn't be touched on the feature branch. I think you're being a little optimistic about there rarely being a need for conflict resolution--often work on a feature requires refactoring/extending the main codebase. Hopefully as we provide a clean (and stable) API between runners and SDKs this will still be the case, but I don't think we're there yet. If there are conflicts, does one check in the conflict markers then resolve in a separate commit? Only for messy ones, or all the time? Certainly if further adjustments are needed we should do a full review. In my opinion, reviewing a merge should not be too laborious a process (e.g. one needn't necessarily read the entire diff as one would with a standard commit). It shouldn't be blocking anyone to wait for a review (one can always develop on top of the merge). If there's not enough activity on a branch to do this, the branch has other troubles. So I'd lean towards not making an exception here, but could be convinced otherwise. - Robert
