[Gluster-devel] Pull Request review workflow

[Sheetal Pamecha]

Regards,
Sheetal Pamecha


On Thu, Oct 15, 2020 at 4:31 PM Amar Tumballi <amar at kadalu.io> wrote:

> Thanks for taking time on this, and sending this note Xavi!
>
> Some comments inline!
>
> On Thu, Oct 15, 2020 at 4:03 PM Xavi Hernandez <xhernandez at redhat.com>
> wrote:
>
>> Hi all,
>>
>> after the recent switch to GitHub, I've seen that reviews that require
>> multiple iterations are hard to follow using the old workflow we were using
>> in Gerrit.
>>
>> Till now we basically amended the commit and pushed it again. Gerrit had
>> a feature to calculate diffs between versions of the patch, so it was
>> relatively easy to follow the changes between iterations (unless there was
>> a big change in the base branch and the patch was rebased).
>>
>> In GitHub we don't have this feature (at least I haven't seen it). So I'm
>> proposing to change this workflow.
>>
>> The idea is to create a PR with the initial commit. When a modification
>> needs to be done as a result of the review, instead of amending the
>> existing commit, we should create a new commit. From the review tool in
>> GitHub it's very easy to check individual commits.
>>
>>
> +1
>
>
>> Once the review is finished, the patch will be merged with the "Squash
>> and Merge" option, that will combine all the commits into a single one
>> before merging, so the end result will be exactly the same we had with
>> Gerrit.
>>
>>
> +1
> Just a note to the maintainers who are merging PRs to have patience and
> check the commit message when there are more than 1 commits in PR.
>
>
> Another thing to consider is that rfc.sh script always does a rebase
>> before pushing changes. This rewrites history and changes all commits of a
>> PR. I think we shouldn't do a rebase in rfc.sh. Only if there are
>> conflicts, I would do a manual rebase and push the changes.
>>
>>
> With github workflow, we don't need './rfc.sh' in my personal opinion. I
> ported it to new branch and github considering the number of developers who
> are used to it.  If you do the changes as per github, then you would have a
> separate branch per PR (ie, feature/bug), so you are at your own to decide
> when to rebase.
>

Actually True, Since the migration to github. I have not been using
./rfc.sh and For me it's easier and cleaner.


> What do you think ?
>>
>>
> I agree, we can remove -f option of ./rfc.sh and also the rebase part in
> ./rfc.sh!
>
> Regards,
> Amar
> --
> https://kadalu.io
>
>
>> Regards,
>>
>> Xavi
>>
>
> _______________________________________________
>
> Community Meeting Calendar:
>
> Schedule -
> Every 2nd and 4th Tuesday at 14:30 IST / 09:00 UTC
> Bridge: https://bluejeans.com/441850968
>
>
>
>
> Gluster-devel mailing list
> Gluster-devel at gluster.org
> https://lists.gluster.org/mailman/listinfo/gluster-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gluster.org/pipermail/gluster-devel/attachments/20201015/0d8ae924/attachment.html>