Re: Pull Request review workflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Regards,
Sheetal Pamecha


On Thu, Oct 15, 2020 at 4:03 PM Xavi Hernandez <xhernandez@xxxxxxxxxx> 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.

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@xxxxxxxxxxx
https://lists.gluster.org/mailman/listinfo/gluster-devel

_______________________________________________

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@xxxxxxxxxxx
https://lists.gluster.org/mailman/listinfo/gluster-devel


[Index of Archives]     [Gluster Users]     [Ceph Users]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux