Working towards a common review format for git

Working towards a common review format for git

[Richard Ipsum]

Hi,

In a prior email I mentioned in passing a library I've been working
on to try to reach a common format for storing review content in git:
perl-notedb.[1]

I'm making reasonable progress with this but my work has uncovered
necessary (and trivial) modifications to Notedb, the first[2] is a
trivial modification to ensure the 'Status' trailer gets written
to the commit when a change's status changes. The second[3] is an RFC
where I suggest adding a 'Commit' trailer so that it is always
possible to reference the commit under review by its sha.

With these patches applied to gerrit it's possible for perl-notedb to parse
all meta content from notedb and map it to the actual git content
under review. However, my concern at present is that I'm already
operating under a fork of notedb which defeats the objective of
collaborating to produce a standard format, let's try to avoid[4]

If the gerrit folks could let me know what I need to do to get
these modifications merged, or else what we need to do to achieve
equivalent functionality I'd really appreciate it.

Thanks again,
Richard Ipsum

[1]: https://bitbucket.org/richardipsum/perl-notedb
[2]: https://gerrit-review.googlesource.com/#/c/73436/
[3]: https://gerrit-review.googlesource.com/#/c/73602/
[4]: https://xkcd.com/927/

Re: Working towards a common review format for git

[Dave Borowitz]

On Fri, Jan 8, 2016 at 9:08 AM, Richard Ipsum
<richard.ipsum@codethink.co.uk> wrote:
> Hi,
>
> In a prior email I mentioned in passing a library I've been working
> on to try to reach a common format for storing review content in git:
> perl-notedb.[1]
>
> I'm making reasonable progress with this but my work has uncovered
> necessary (and trivial) modifications to Notedb, the first[2] is a
> trivial modification to ensure the 'Status' trailer gets written
> to the commit when a change's status changes.

I would consider this a bugfix, and will respond on that review.

> The second[3] is an RFC
> where I suggest adding a 'Commit' trailer so that it is always
> possible to reference the commit under review by its sha.

I think this is probably fine but I'll have to think about it some more.

> With these patches applied to gerrit it's possible for perl-notedb to parse
> all meta content from notedb and map it to the actual git content
> under review. However, my concern at present is that I'm already
> operating under a fork of notedb which defeats the objective of
> collaborating to produce a standard format, let's try to avoid[4]

I hope I can assuage some of your concerns by saying that since Gerrit
notedb is such a work in progress, literally nobody is running it in
the wild, so even if the formats diverge temporarily I don't see it as
being a long-term issue. But thank you for caring about it, I do
appreciate your proactive considerations.

> If the gerrit folks could let me know what I need to do to get
> these modifications merged, or else what we need to do to achieve
> equivalent functionality I'd really appreciate it.
>
> Thanks again,
> Richard Ipsum
>
> [1]: https://bitbucket.org/richardipsum/perl-notedb
> [2]: https://gerrit-review.googlesource.com/#/c/73436/
> [3]: https://gerrit-review.googlesource.com/#/c/73602/
> [4]: https://xkcd.com/927/

Re: Working towards a common review format for git

[Dave Borowitz]

On Fri, Jan 8, 2016 at 9:24 AM, Dave Borowitz <dborowitz@google.com> wrote:
> On Fri, Jan 8, 2016 at 9:08 AM, Richard Ipsum
> <richard.ipsum@codethink.co.uk> wrote:
>> Hi,
>>
>> In a prior email I mentioned in passing a library I've been working
>> on to try to reach a common format for storing review content in git:
>> perl-notedb.[1]
>>
>> I'm making reasonable progress with this but my work has uncovered
>> necessary (and trivial) modifications to Notedb, the first[2] is a
>> trivial modification to ensure the 'Status' trailer gets written
>> to the commit when a change's status changes.
>
> I would consider this a bugfix, and will respond on that review.
>
>> The second[3] is an RFC
>> where I suggest adding a 'Commit' trailer so that it is always
>> possible to reference the commit under review by its sha.
>
> I think this is probably fine but I'll have to think about it some more.
>
>> With these patches applied to gerrit it's possible for perl-notedb to parse
>> all meta content from notedb and map it to the actual git content
>> under review. However, my concern at present is that I'm already
>> operating under a fork of notedb which defeats the objective of
>> collaborating to produce a standard format, let's try to avoid[4]
>
> I hope I can assuage some of your concerns by saying that since Gerrit
> notedb is such a work in progress, literally nobody is running it in
> the wild, so even if the formats diverge temporarily I don't see it as
> being a long-term issue. But thank you for caring about it, I do
> appreciate your proactive considerations.

You know what would probably also be a good idea would be to spec out
the entire format in a standalone document. That way when Gerrit
doesn't have something implemented, it's clear that Gerrit is wrong,
not that it's trying to fork the format.

>> If the gerrit folks could let me know what I need to do to get
>> these modifications merged, or else what we need to do to achieve
>> equivalent functionality I'd really appreciate it.
>>
>> Thanks again,
>> Richard Ipsum
>>
>> [1]: https://bitbucket.org/richardipsum/perl-notedb
>> [2]: https://gerrit-review.googlesource.com/#/c/73436/
>> [3]: https://gerrit-review.googlesource.com/#/c/73602/
>> [4]: https://xkcd.com/927/

Re: Working towards a common review format for git

[Richard Ipsum]

On Fri, Jan 08, 2016 at 09:53:53AM -0500, Dave Borowitz wrote:
> On Fri, Jan 8, 2016 at 9:24 AM, Dave Borowitz <dborowitz@google.com> wrote:
> > On Fri, Jan 8, 2016 at 9:08 AM, Richard Ipsum
> > <richard.ipsum@codethink.co.uk> wrote:
> >> Hi,
> >>
> >> In a prior email I mentioned in passing a library I've been working
> >> on to try to reach a common format for storing review content in git:
> >> perl-notedb.[1]
> >>
> >> I'm making reasonable progress with this but my work has uncovered
> >> necessary (and trivial) modifications to Notedb, the first[2] is a
> >> trivial modification to ensure the 'Status' trailer gets written
> >> to the commit when a change's status changes.
> >
> > I would consider this a bugfix, and will respond on that review.
> >
> >> The second[3] is an RFC
> >> where I suggest adding a 'Commit' trailer so that it is always
> >> possible to reference the commit under review by its sha.
> >
> > I think this is probably fine but I'll have to think about it some more.
> >
> >> With these patches applied to gerrit it's possible for perl-notedb to parse
> >> all meta content from notedb and map it to the actual git content
> >> under review. However, my concern at present is that I'm already
> >> operating under a fork of notedb which defeats the objective of
> >> collaborating to produce a standard format, let's try to avoid[4]
> >
> > I hope I can assuage some of your concerns by saying that since Gerrit
> > notedb is such a work in progress, literally nobody is running it in
> > the wild, so even if the formats diverge temporarily I don't see it as
> > being a long-term issue. But thank you for caring about it, I do
> > appreciate your proactive considerations.

Thanks, that's good to know,
thanks also for reviewing the modifications I mentioned. :)

> 
> You know what would probably also be a good idea would be to spec out
> the entire format in a standalone document. That way when Gerrit
> doesn't have something implemented, it's clear that Gerrit is wrong,
> not that it's trying to fork the format.

I think that would be good, I could attempt to write a spec for the format
and post it here for review? I expect I'll want to work a little more on
perl-notedb before I do that though.

Re: Working towards a common review format for git

[Dave Borowitz]

On Fri, Jan 8, 2016 at 2:23 PM, Richard Ipsum
<richard.ipsum@codethink.co.uk> wrote:
>> You know what would probably also be a good idea would be to spec out
>> the entire format in a standalone document. That way when Gerrit
>> doesn't have something implemented, it's clear that Gerrit is wrong,
>> not that it's trying to fork the format.
>
> I think that would be good, I could attempt to write a spec for the format
> and post it here for review? I expect I'll want to work a little more on
> perl-notedb before I do that though.

Sure. I don't want to necessarily make more work for you, but it's
probably a good exercise to have someone write a spec who did not also
write the first implementation :)

You may post it here but you would probably get more feedback on
repo-discuss@googlegroups.com.