Re: [PATCH 0/2] git-candidate: git based patch tracking and review

[Richard Ipsum]

On Tue, Dec 01, 2015 at 04:00:52PM -0500, Dave Borowitz wrote:
> On Tue, Dec 1, 2015 at 3:55 PM, Jonathan Nieder  wrote:
> > Cc-ing dborowitz, who has been working on storing Gerrit's code review
> > information in Git instead of a separate database (e.g., see [1]).
> 
> Thanks, we actually already had a thread going that I realize only in
> retrospect did not include the git mailing list.

Thanks, we did indeed have a thread going,
that probably should have been on-list.
I'm also working on a little library[1] that should eventually allow other
tools to use gerrit's notedb in the hope that this might
eventually lead to the development/adoption of a common format for such tools.

One thing that concerns me about notedb with respect to distributed review
is the storage format for comments. Within a distributed review system comments
may be made in any order, yet the format is designed around the kind of
linearisation that can be assumed by a centralised system.

The problem is that multiple comments in notedb may be stored within
the same blob/note, specifically all comments on a particular commit will
be stored in the same blob. In a distributed system storing multiple comments
in the same blob like this will inevitably lead to merge conflicts.

This problem isn't unsolvable, someone already suggested to me the idea
of writing a custom merge driver for merging different notes stored in notedb.
It would obviously be preferable to have a format that avoided creating
conflicts in the first place, but a custom merge driver doesn't seem like
an unreasonable solution.

If anyone has any thoughts on how else this problem might be solved,
I'd be very interested to hear them.

Thanks again,
Richard Ipsum

[1]: https://bitbucket.org/richardipsum/perl-notedb
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] git-candidate: git based patch tracking and review

[Jonathan Nieder]

Michael Haggerty wrote:
> On 11/10/2015 01:56 PM, Richard Ipsum wrote:

>> I've continued my work[1] to add patch tracking and candidate review 
>> capability
>> to git.
>>
>> git-candidate now has a more git-like user interface, so remote candidates
>> can now be specified in a similar way to remote refs (e.g. origin/candidate)
>> as well as various other improvements, such as versioned metadata.
>
> This is a really interesting project. I've seen a blog post or two
> proposing to store bug tracker information in Git in a distributed way,
> but I don't recall anything about doing the same for code review
> information.

Cc-ing dborowitz, who has been working on storing Gerrit's code review
information in Git instead of a separate database (e.g., see [1]).

Thanks,
Jonathan

[1] https://storage.googleapis.com/gerrit-talks/summit/2015/NoteDB.pdf
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] git-candidate: git based patch tracking and review

[Dave Borowitz]

On Tue, Dec 1, 2015 at 3:55 PM, Jonathan Nieder  wrote:
> Cc-ing dborowitz, who has been working on storing Gerrit's code review
> information in Git instead of a separate database (e.g., see [1]).

Thanks, we actually already had a thread going that I realize only in
retrospect did not include the git mailing list.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] git-candidate: git based patch tracking and review

[Jeff King]

On Wed, Nov 11, 2015 at 03:12:05PM +, Richard Ipsum wrote:

> > All that being said, my gut feeling is that a system like this should
> > not be developed within the Git project itself. Code review is a
> > complicated thing, and I expect that different people will have very
> > different ideas about how it should work. It would be a bad idea for the
> > Git project to "bless" one system by including it in our source tree.
> > (Earlier in the Git's history it was easier to get something accepted
> > into "contrib", but that has gotten much harder over time.)
> 
> The aim is not to bless one particular system but to eventually
> provide a common data model that all review systems can share,
> so that it is possible to do distributed reviews with arbitrary UIs
> in a widely compatible way.

I think that's a laudable goal, but I didn't see any discussion or
documentation of the data model in your patches. Maybe that would be a
good place to start.

> If we add git-candidate to contrib then it can act as a reference
> implementation, so that this data model can be validated and tested
> by additional developers.

That can happen outside of git's contrib/ directory, too.

I think Michael's "bless" argument applies to the data model, too. Is
your data model a good one? Should other systems adopt it, or is it
still a work in progress? We don't know yet.

I think I'd rather see it prove itself before entering the git tree, if
only because it doesn't really gain anything by being inside the git
tree. Once upon a time that was a good way to get publicity and easy
hosting, but these days it is easy to find git hosting, and I am not
sure people actually explore contrib/ all that much.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] git-candidate: git based patch tracking and review

[Junio C Hamano]

Jeff King  writes:

> On Wed, Nov 11, 2015 at 03:12:05PM +, Richard Ipsum wrote:
>> 
>> The aim is not to bless one particular system but to eventually
>> provide a common data model that all review systems can share,
>> so that it is possible to do distributed reviews with arbitrary UIs
>> in a widely compatible way.
>
> I think that's a laudable goal, but I didn't see any discussion or
> documentation of the data model in your patches. Maybe that would be a
> good place to start.

Even before doing that it might be a good idea to pick brains of
folks who work on Gerrit, who has already done one such data model
that may be generic enough to serve as a good starting point. ISTR
that they were migrating to a notes based storage to ease federated
code review systems?

>> If we add git-candidate to contrib then it can act as a reference
>> implementation, so that this data model can be validated and tested
>> by additional developers.
>
> That can happen outside of git's contrib/ directory, too.
>
> I think Michael's "bless" argument applies to the data model, too. Is
> your data model a good one? Should other systems adopt it, or is it
> still a work in progress? We don't know yet.
>
> I think I'd rather see it prove itself before entering the git tree, if
> only because it doesn't really gain anything by being inside the git
> tree. Once upon a time that was a good way to get publicity and easy
> hosting, but these days it is easy to find git hosting, and I am not
> sure people actually explore contrib/ all that much.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] git-candidate: git based patch tracking and review

[Michael Haggerty]

On 11/10/2015 01:56 PM, Richard Ipsum wrote:
> I've continued my work[1] to add patch tracking and candidate review 
> capability
> to git.
> 
> git-candidate now has a more git-like user interface, so remote candidates
> can now be specified in a similar way to remote refs (e.g. origin/candidate)
> as well as various other improvements, such as versioned metadata.

This is a really interesting project. I've seen a blog post or two
proposing to store bug tracker information in Git in a distributed way,
but I don't recall anything about doing the same for code review
information.

I would be interested to hear about the design of your system at an
abstract technical level. What do you store in Git and in what layout?
Do you need to record any extra metadata within the commits that are
merged to master? How do you merge and/or reconcile code review comments
that come from multiple sources (or are they just tabulated)? Can your
system handle the rebasing of topic branches? What about nonlinear topic
branches (branches branches that themselves include merges)?

All that being said, my gut feeling is that a system like this should
not be developed within the Git project itself. Code review is a
complicated thing, and I expect that different people will have very
different ideas about how it should work. It would be a bad idea for the
Git project to "bless" one system by including it in our source tree.
(Earlier in the Git's history it was easier to get something accepted
into "contrib", but that has gotten much harder over time.)

If, someday, one system becomes crushingly dominant, then conceivably it
would make sense for it to be distributed along with Git for the
convenience of users. Or if a bunch of review systems standardize on a
single data model for storing review information in a Git repo, it might
make sense for the plumbing for handling that data to reside in git-core
for performance and data integrity reasons. Until then, I think it would
be better for code review systems to live on their own, as independent
projects.

In my opinion it would be fine to discuss the design of your system and
solicit feedback about the design on the Git mailing list, and also to
publish occasional announcement emails when you release new versions or
whatever. You might also want to list your system on the Git SCM wiki,
for example here [1].

Yours,
Michael

[1] https://git.wiki.kernel.org/index.php/Interfaces,_frontends,_and_tools

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html