On Thu, Aug 8, 2013 at 1:46 PM, Sage Weil <[email protected]> wrote: > On Thu, 8 Aug 2013, Loic Dachary wrote: >> Hi Sage, >> >> During the discussions about continuous integration at the CDS this week ( >> http://youtu.be/cGosx5zD4FM?t=1h16m05s ) you mentionned that github was able >> to keep track of the successive versions of a pull request commit, even in >> the case of a rebase. I just tried the following: >> >> a) comment on commit associated to https://github.com/ceph/ceph/pull/455 ( >> both inline and at the end of the commit ) >> b) Christophe rebased the associated commit against master and git push >> --force to publish it >> c) The original commit does not show up in >> https://github.com/ceph/ceph/pull/455 and the new commit, result of the >> rebase, >> https://github.com/kri5/ceph/commit/051b0c3e15c98714b95fca5cb7838de9614dc8e3 >> does not display my inline comments. >> >> Do you to know a different workflow that would allow rebase and keep track >> of the different versions / comments of the pull request ? Something similar >> to what you have in gerrit with https://review.openstack.org/#/c/22708/ for >> example where you can see and compare the patch sets. > > Well, blarney. Yeah, it is not perfect. > > The workflow that *does* work: > > - open a pull > - comment on the rolled-up diff, e.g. > https://github.com/ceph/ceph/pull/487/files > - re-push teh branch > > and it will do a cute 'so and so commented on 3 old commits' thing. > > This isn't always how people like to review, sadly. I'm hopeful that > github will suddenly get better and this problem will go way, but > shouldn't hold my breath. > > In my mind this isn't a deal-breaker... whatdoes everyone else think?
It's fairly frustrating when reviewing large branches. What I've resorted to is doing reviews in gitk on a patch-by-patch basis and then going and finding the line in the rolled-up diff. That does work, but it's really awkward, especially when the full patch set includes changes that don't fix an issue but do obscure its origin. I would love a more robust review system but the more I hear about Gerrit the less it sounds like I could stand that either. :/ -Greg Software Engineer #42 @ http://inktank.com | http://ceph.com -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
