yeah...i could have been hasty writing " Avoid manual merges for most things and prefer the "Merge PR" button". The keyword there being "most".....just use multiple pull requests as needed as we have been doing.
On Tue, Oct 16, 2018 at 7:00 AM Jorge Bay Gondra <[email protected]> wrote: > 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? > > >> > >> > > >> > > > > >> > > > >> > > > > > >
