reviewers modifying code they are reviewing

[William Hubbs]

Hi all,

I want to bring up something I'm very concerned about that happened to
me today.

I feel that when I submit patches to this list, if they need to be
modified, or fixed, the reviewer should write back to me on the list and
explain what the issues are, so that we can discuss them BEFORE any
changes are made to the code I have submitted, and especially before
those changes are committed to the tree.

The below is being used just to point out my issue.  I am not angry with
Andrei, we worked out the situation fine.  I'm just bringing this to the
list to show my concerns.

On Sun, Sep 19, 2010 at 04:23:23PM +0200, Andrei Kholodnyi wrote:
> > Thanks William.
> > This series is pushed with a minor modifications.
> 
> William has asked me to describe modifications I did, so here we go.

I asked him to describe the modifications because he did not discuss
any changes with me before commiting them to the tree, so I had no idea
what he had changed.

> there was one bug in your patch
> - $(inc_audio)$(ibmtts_include)  were written together

Like I told him, this was a good catch, and I appreciate him fixing it.

> - you have added $(top_srcdir)/include in clients and tests and I have
> removed it since these are client programs.

This also is correct, and I appreciate him fixing it.

> - I have also used INCLUDES instead of inc_local where applicable.

I asked why this was done since using the inc_local variable was
working.  I asked Andrei about the INCLUDES variable, since I hadn't
seen it used before and was not familiar with it.  We both looked this
variable up in the automake manual and discovered that it is deprecated
and should have never been used.

This has been fixed as of a few minutes ago, so there is nothing to
worry about in the tree currently.

Again, I do not have any bad feelings toward Andrei over this; we worked
that out fine.  I'm bringing it up purely as a procedural concern.

Committers/reviewers, can you please remember to communicate with the
contributors of the code to be sure how they feel about modifications to
their contributions before you modify their code?

Thanks much,

William

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: 
<http://lists.freebsoft.org/pipermail/speechd/attachments/20100919/6a432e49/attachment.pgp>