Re: Patch review

"Stefan Kaltenbrunner"  writes:

> Tom Lane wrote:
>> Magnus Hagander  writes:
>>> Not sure if this is platform-specific, but I keep being annoyed by not being
>>> able to actually *view* the patches in the queue. I have to download them,
>>> and then open them separately. I can't just view them in the browser,
>>> because they're all named ".bin" and come out as mime type
>>> "application/octet-stream".
>>
>> Yeah, it happens that way for me too.  The other huge, huge problem with
>> it is the lack of stability of URLs for the items in the list, which
>> makes it difficult to identify which item you're talking about.
>
> yeah that is fairly annoying :-(

Strangely enough I have the opposite complaint. Sometimes the patch is
displayed inline (seems to be pretty unpredictable) and that makes it hard to
download and apply. If you download the page you have to un-html-escape it and
if you copy-paste the contents the whitespace is messed up.

It would be nice if the patches list were fed into reviewboard or viewcvs or
something like it. Even if we only used it for viewing patches it would at
least make that nice.

I couldn't see how to make it work with reviewboard at first glance. It seems
to assume you're starting form a working tree and wants to generate the diff
and send it in itself. If you generate it yourself then you have to tell it
precisely what it's a diff against. It doesn't seem to expect to be used as a
general purpose patch viewer.

>> Personally I'd be happier with an editable wiki page consisting of links
>> to the original messages in the mail list archives, plus free-format
>> annotations (such as status).  This should be trivial to set up and
>> reasonably flexible.  With experience we might find we need something
>> fancier, but let's not overdesign our solution at the start.
>
> I will see if I can come up with a proposal on the developer wiki for a list
> that looks like that tomorrow.

How powerful are these wiki things? Can you write a script which automatically
inserts a new line in a table in the wiki whenever an email to the patches
list containing a patch comes in?

Can you add buttons on the wiki itself to do common actions like removing the
item from the table or joining it up with another item? That way we would only
have to actually edit the wiki content if we have something substantive to add
such as review comments.

> _IF_ we are looking for an (over)designed solution

I'm against overdesigned solutions. If we want more than a wiki I think it
would be reviewboard. I think something which fills a specific hole like
reviewboard is much more likely to work in the long term than an overarching
solution whose problem description is as vague as a "tracker". As soon as our
process changes the "tracker" will be obsolete whereas a problem-specific
solution like reviewboard will always be useful as long as there are patches
to review.

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training!