[Gluster-devel] Pull Request review workflow

Sheetal Pamecha spamecha at redhat.com
Thu Oct 15 11:01:38 UTC 2020

+1 for this. I too face the issue with losing old changes once a new commit
is pushed.

Sheetal Pamecha

On Thu, Oct 15, 2020 at 4:03 PM Xavi Hernandez <xhernandez at redhat.com>

> 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.
> 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.
> 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.
> What do you think ?
> 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/6714d583/attachment.html>

More information about the Gluster-devel mailing list