Re: September 2015 Commitfest

Hello,

>> +1. FWIW, I'm willing to review some patches for this CommitFest, but
>> if the committers have to do first-round review as well as
>> committer-review of every patch in the CommitFest, this is going to be
>> long, ugly, and painful. We need to have a substantial pool of
>> non-committers involved in the review process so that at least some of
>> the work gets spread out.
>
> As a non-committer, let me offer my thoughts.
>
> First, I would ask that every patch include a commit id that the patch
> was generated against. For example, I was looking at the "group command
> option for psql" patch. I created a branch off of master, but the patch
> doesn't apply cleanly. On further investigation, it looks like Adam
> Brightwell noted this on September 21, but the patch hasn't been
> updated. If I knew which commit id the patch was created against, I
> could create a branch from there and test the patch, but without, I need
> to figure that out which is more work, or I need to re-create the patch,
> which is also more work.

+ 1

> Second, it would be convenient if there were a make target that would
> set up a test environment. Effectively do what the 'make check' does,
> but don't run the tests and leave the database up. It should probably
> drop you into a shell that has the paths set up as well. Another
> target should be available to shut it down. So, what would be cool,
> and make testing easier is if I could do a 'git checkout -b feature
> abcdef' (or whatever the commit id is and branch name you want to use)
> Then from there a
>
> make
> make check
> make testrig
> <whatever tests you want to do>
> make testshutdown
>
> These two would go a long way to making the process of actually
> doing a review easier.

From my point of view it is very hard to review a patch without having
much time. My C knowledge is very very basic. I read many patches just
to get better with this, but as you (not) noticed, there is rarely
feedback from me.

Often it is unclear what to test. The threads about the features are
very long and mostly very technical. While interesting for a good
C-programmer this is not helpful for an end user. When there is a patch
i am missing:
- a short description how to set up an test installation (just a link to
the wiki would be very helpful)
- what is the patch proposed to do
- what should it not do
- how can it be tested
- how can the updated documentation - if existent - generated and used

Often terms, configuration options or syntax changes between the
patches. This is needed and very good - but currently requires me to
read the complete thread, which has many many information i do not
understand because of the missing inside.

I believe that we need to lower the barrier for testing. This would
require another step of work. But this step is not necessarily done by
the author itself. I am under the impression that there are many readers
at the mailing list willing but unable to participate. This could be a
"grunt-work" task like in this discussion about the bugtracker.

I could even imagine to set up an open for everyone test-instance of
HEAD where users are allowed to test like the wanted. Than the barrier
is reduced to "connect to PostgreSQL and execute SQL".

Greetings,
Torsten