I think we should continue using the same process, creating a pull request against the correct branch, merge it (using button or manually) and merge upstream manually.
Using separate pull requests for each branch creates an unnecessary overhead / noise in most cases. The main disadvantage is the extra work to be done by community contributors and committers. Taking master as a superset of changes of the version branches, I think a merge commit from tp33 to master is enough. In case we need to trigger CI builds and check how conflict resolution went for tougher merges, we can always create a separate pull request or create a separate branch and merge that after CI finished, for example: git checkout -b master-upstream master git merge tp33 # resolve conflicts and commit, then push to origin git push origin master-upstream # wait for ci to finish, then merge fast-forward git checkout master git merge --ff-only master-upstream git push origin master El lun., 8 oct. 2018 a las 22:35, Stephen Mallette (<[email protected]>) escribió: > sorry, I spoke somewhat incorrectly when i wrote "no weird secondary merge > commits" - you still get what basically amounts to an empty merge between > release branches taking this approach. > > On Mon, Oct 8, 2018 at 4:01 PM Stephen Mallette <[email protected]> > wrote: > > > Good questions... > > > > > 1. we would be pushing the original commits and each merge as PRs? > > > > Here's what I just did moments ago. We started with this: > > > > https://github.com/apache/tinkerpop/pull/952 [tp32 branch] > > > > That originally came from a third-party. We reviewed and voted that. When > > it was "ok" (i.e. three +1s in this case) I opted to create PRs for the > > other branches: > > > > https://github.com/apache/tinkerpop/pull/956 [tp33] > > https://github.com/apache/tinkerpop/pull/957 [master] > > > > why? because: > > > > 1. i can see exactly what's going into the release branches through > github > > 2. i get travis builds and they will merge as-viewed/as-tested with the > > "Merge Pull Request" button > > 3. because i use the "Merge Pull Request" button there's no weird > > secondary merge commits and even less chance for having git in a weird > > state for another committer when they pull > > > > Do we always have to follow this pattern for every PR? I think we can > > leave that to the discretion of the committer. Sometimes for a small > change > > it may not be worth the energy and you can just command line all of it. > In > > other cases, where there are conflicts and such, it might be better to > > follow this approach. Btw, Travis has been much more stable in recent > > months (my opinion) as I've been slowly tweaking tests that have been > > non-deterministic in the past and it's finally getting to a point where a > > failure might actually mean something AND now that we can force rebuild > > directly with a button push....that's even more incentive to go this > route. > > > > > 2. do we have to VOTE on each branch PR for the same issue? > > > > I'd say "no", unless the committer wants extra review - e.g. something > > that was heavily conflicted. > > > > > 2a. what if they have different vote results? does one block the > other? > > > > i suppose that could happen but it's never happened yet. it would be like > > there was a change in tp32 that we didn't want in tp33. since that has > > never happened i don't know what we'd do but discuss it and come to some > > consensus on how to proceed. i think that's what we'd do now if that > every > > occurred. > > > > > > > > On Mon, Oct 8, 2018 at 3:38 PM Robert Dale <[email protected]> wrote: > > > >> So, > >> 1. we would be pushing the original commits and each merge as PRs? > >> 2. do we have to VOTE on each branch PR for the same issue? > >> 2a. what if they have different vote results? does one block the other? > >> > >> Robert Dale > >> > >> > >> On Mon, Oct 8, 2018 at 11:52 AM Stephen Mallette <[email protected]> > >> wrote: > >> > >> > I made a handful of fixes to dev docs - reflects some of the things I > >> wrote > >> > on this thread: > >> > > >> > > >> > > >> > https://github.com/apache/tinkerpop/commit/3ec14cbad6abc0483bae91775956ce6af812627a > >> > > >> > we can tweak further as necessary - thanks > >> > > >> > > >> > On Mon, Oct 8, 2018 at 9:04 AM Stephen Mallette <[email protected] > > > >> > wrote: > >> > > >> > > wow - the "Restart Build" button in Travis works now and of course > the > >> > > "Close Pull Request" button is enabled. no more "git commit > >> > > --allow-empty"............. > >> > > > >> > > On Mon, Oct 8, 2018 at 8:19 AM Stephen Mallette < > [email protected] > >> > > >> > > wrote: > >> > > > >> > >> Just used the "Merge Pull Request" button now that we have GitBox > and > >> > >> that felt good. > >> > >> > >> > >> Now that we have push access to github, I believe we want some > >> changes > >> > to > >> > >> our git flow. Here's some thoughts: > >> > >> > >> > >> 1. Avoid manual merges for most things and prefer the "Merge PR" > >> button > >> > >> in GitHub > >> > >> 2. Issue PRs to each branch that will be updated > >> > >> 3. Use the "Merge PR" button in reverse version order (as we should > >> be > >> > >> doing now with manual merges - merge master first, then tp33 and > then > >> > tp32) > >> > >> 4. I suppose that for external PRs we could ask the submitter to > >> issue > >> > >> multiple PRs depending on the base branch they are targeting, but > >> that > >> > >> might create some extra friction for new contributors. Maybe we > >> > committers > >> > >> should just create those PRs for them in those cases (unless they > are > >> > >> conflicted badly then perhaps the submitter should go back and deal > >> with > >> > >> that). > >> > >> 5. CTRs will still need manual merging so I guess that doesn't go > >> away. > >> > >> > >> > >> Any other ideas? > >> > >> > >> > > > >> > > >> > > >
