Hey Enrico,

I think we can do better than that stacked PR blog says. GH now allows
you to change the target of a PR which wasn't the case when that was
written. I think we could even start using branches in the main repo,
and delete them after we merge, and set the base of the pull request
to be the previous patch in the series.

I've put up an example, and commands I used are here:
https://gist.github.com/ivankelly/bfe16c81156dc3be7123921f37a6274d

Each PR can be reviewed individually. Any changes for a PR can be
pushed for that branch. If conflicts occur, you can rebase or merge to
fix it. In the later patches.

When a patch is merged, any pull requests that depend on it can have
it's base changed to master, and then the branch can be deleted.

It should be straightforward to add a script to do this to avoid the typing.

Something like.
dev/bk-stacked-pr.py -h foobar -n ivan/my-feature

Where -h is the sha1 head of list of commits to create a pull request
for, and -n is the basename of the branches that will be created.

Unfortunately this would only be possible for committers, but
something similar could be done with private forks, though the reviews
wouldn't be as visible.

The base changing, branch cleanup can be done in bk-merge-pr.py.

-Ivan

On Fri, Oct 20, 2017 at 8:19 AM, Enrico Olivelli <eolive...@gmail.com> wrote:
> I have created a chain of stacked Pull Requests using my private fork
> (eolivelli/bookkeeper)
>
> https://github.com/eolivelli/bookkeeper/pulls
>
> https://github.com/eolivelli/bookkeeper/pull/2 part 1
> https://github.com/eolivelli/bookkeeper/pull/3 part 2
> https://github.com/eolivelli/bookkeeper/pull/4 part 3
>
> Each Pull Request is against the previous one:
>
> Pros:
> - this makes it very simple to handle diffs visually and so to comment on
> specific changes
> - when I change a line in PR1, simply I use "merge" (git checkout
> PR2-branch, git merge PR1-branch, git checkout PR3-branch, git merge
> PR3-branch)
>
> Cons:
> - the conversation will be hosted entirely on my GitHub fork, non on the
> Apache one
>
>
> The alternative approach is to have separate Pull Requests on Apache repo,
> like these
>
> https://github.com/apache/bookkeeper/pull/642
> https://github.com/apache/bookkeeper/pull/643
>
> Pros:
> - the conversation is on Apache stuff
>
> Cons:
> - the developer (me in this case) has to fix all the commits and perform
> clean up in order to port changes in downstream pull requests to upstream
> once
>
>
>
> I hope we can find a good way of working on this kind on "huge changes" and
> let "reviewers" and "contributors" do their work without wasting precious
> time
>
> As a contributor in this case I prefer approach 1), because life is really
> easy, maybe I am not a git guru and there is a better way to handle
> approach 2)
>
> There is a 1 and 1/2 approach, that is to do 1) but with branches on Apache
> repo, but Ivan suggested to use private forks (and I agree with Ivan)
>
> Thank you all (expecially Sijie and Ivan) for you help and support, I do
> not want to waste reviewers' time, It takes time and patience to "really"
> review code
>
> Thoughts ?
>
> Enrico
>
>
>
>
> 2017-10-18 17:38 GMT+02:00 Ivan Kelly <iv...@apache.org>:
>
>> For branching, I would create the on your own personal fork.
>>
>> -Ivan
>>
>> On Wed, Oct 18, 2017 at 8:37 AM, Ivan Kelly <iv...@apache.org> wrote:
>> > I had a quick look, I'll get to reviewing today (in bay area this
>> > week, so day is just starting), but looks great.
>> >
>> > -Ivan
>> >
>> > On Wed, Oct 18, 2017 at 8:31 AM, Enrico Olivelli <eolive...@gmail.com>
>> wrote:
>> >> Hi,
>> >> I am trying to split the patch for BP-14 in multiple pieces
>> >>
>> >> This is the umbrella issue, I will link here all the Pull Requests
>> >> https://github.com/apache/bookkeeper/issues/471
>> >>
>> >> I am actually in work-in-progress, I have covered metadata and client
>> side
>> >> changes with the first 2 PRs.
>> >>
>> >> It will take some more days to create all of the PRs.
>> >>
>> >> In order to make it simpler to create stacked PRs it would be better to
>> >> create branches and PRs not against the master but every PR is to be
>> sent
>> >> against the previous one in the list, see [1] as Ivan suggested.
>> >>
>> >> I wounder if it would be better to use my own github repo or to create
>> >> branches on Apache repo.
>> >> I see many projects are using branches directly on apache repository,
>> for
>> >> instance Apache Maven.
>> >> I am not a fan of this because it would lead to some kind of pollution
>> in
>> >> the primary repo.
>> >>
>> >> I will try to set up the whole thing in my repo and come back maybe
>> with a
>> >> better approach.
>> >>
>> >> For me it is a real experiment I hope I am not creating too much noise
>> >>
>> >> Cheers
>> >> Enrico
>> >>
>> >> [1]
>> >> https://graysonkoonce.com/stacked-pull-requests-keeping-
>> github-diffs-small/
>>

Reply via email to