+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 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".


Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to