New contributors?

["Susan L. Cline" <ho...@pacbell.net>]

Hello Derby-dev, 

I'm writing to point out an experience which discouraged me about growing our community with new
contributors. 

Recently I submitted a patch for a JUnit test.   I have contributed very few patches in the form
of code to Derby (I've had just a couple for the Eclipse plug-ins.)  I am not a Java developer by
trade or training, but I have contributed to the Derby community by writing 
articles, sample applications and some wiki entries. 

My JUnit contribution received comments by two different folks.  I believe that some of the
comments were valid and helpful to me to improve my programming skills and also to help insure
that the test I wrote was readable and understandable, as well as to fit into the constraints that
JUnit imposes as well as the Derby test harness. 

However, I feel that some of the comments were more style-related or were nits that the individual
reviewer had.  This is where I feel discouraged.  One of the biggest goals I think we have is to
embrace contributors (and therefore their contributions) and to guide them 
where it is necessary, but also to accept contributions that represent "incremental work."  Also,
I think when someone makes a contribution it has something to do with that individuals "itch".  In
this case I thought it would be fun to learn JUnit and become more familiar with the 
Derby test harness, and that was my itch.   

I am fortunate in that I am paid to work on Derby, but I think of other individuals who are not
paid to work on Derby.  If they make a contribution which is not accepted, partially due to style
issues, how likely are we to get new contributors?   Again, I'm not saying that I wrote the
perfect test and others are not welcome to comment on my code, but on the other hand if we make
the barrier to contribute (and it was just a test!) this high, how likely are we to get
contributions from individuals who have a lesser stake in Derby than their own livelihood? 

Thanks, 

Susan 

Re: New contributors?

["Jean T. Anderson" <jt...@bristowhill.com>]

Jean T. Anderson wrote:
> Daniel John Debrunner wrote:
>>Jean T. Anderson wrote:
>>>Daniel John Debrunner wrote:
>>>>Jean T. Anderson wrote:
...
> In other words, as a reviewer state how much you care that a particular
> item be fixed.
> 
>>>To take a concrete example .....
>>>
>>>I won't commit a doc patch unless:
>>>
>>>- Somebody indicates the changes are technically ok.
>>>- The DITA doc build succeeds.
>>
>>But my (or someone else's) confidence level *might* be lower, if the doc
>>builds then it's ok, better to get more eyes on it earlier, thus I
>>commit it, even if you don't have confidence.
>  
> That's fine -- I view it as an individual committer definition. *I*
> (Jean) won't commit a doc patch unless ... [insert the bullets from the
> above].  I don't expect others to adopt my criteria.
> 
> That much said, I'll be kind of annoyed if somebody does a commit and
> the doc build fails.  :-) And only because I know how time consuming it
> is to chase down doc build problems.

in reference to my saying "I'll be kind of annoyed if somebody does a
commit and the doc build fails" somebody pointed out to me off list that
it is "the kind of comment that can discourage contributors".

Sorry, that wasn't my intention. :-) As the one who recently committed a
patch that broke the build, I can honestly say that, yes, I was annoyed
at myself. But I didn't yell at myself on list, I didn't fall on my
sword. I resolved the problem -- and developed an excellent appreciation
for how difficult it can be to chase down doc build problems... hence my
own personal rule that I won't commit a patch unless the DITA doc build
succeeds.

So, I believe I can help contributors by setting a base level
expectation that the doc build needs to succeed. Furthermore, if the
patch addresses a technical area with which I'm not familiar, I feel the
most confident when somebody with expertise in that area reviews it. I
think my stating my own game plan will encourage contributors and
hopefully reduce that contributor's sense of jumping through hoops.

I could take that further and add some words of encouragement, which are
that you don't necessarily need to know DITA to edit the documentation.

Some contributors *do* know DITA, including Andrew, Laura, Halley, Kim,
and Stan (if I missed your name -- sorry!), and their contributions are
greatly appreciated.

But I'll say straight up that I don't know DITA. I do know html and xml,
so I can modify existing text in the DITA source and troubleshoot
problems with tags that aren't closed. However, I would be kind of lost
if I wanted to add a new topic -- and I'd ask for help getting started.

I don't know how much DITA Mayuresh, Myrna, and Bryan know, but their
cleanups to the Reference Guide and Working With Derby (DERBY-1072,
DERBY-1821, DERBY-1765) are vast improvements that move the
documentation forward for everyone.

I don't know how much DITA Kristian Waagan knows, but his offering to
take a stab at the documentation for DERBY-1659 is fabulous and I
applaud his approach: "If someone wants to add to the content after the
first patch is submitted, feel free to assign yourself to the issue and
bring it forward."

In short, thanks to everyone for improving the docs -- it is sincerely
appreciated! And if I missed thanking you by name in this post, please
let me know.

 -jean

Re: New contributors?

["Jean T. Anderson" <jt...@bristowhill.com>]

Daniel John Debrunner wrote:
>Jean T. Anderson wrote:
>>Daniel John Debrunner wrote:
>>>Jean T. Anderson wrote:
>>>
>>>>It might be helpful for reviewers to clearly separate "must fix" items
>>>>from "nice to have" items in their comments.
>>>
>>>Sounds great, but what is a must fix? 
>>
>>A simple definition could be anything that would prevent it from being
>>committed. If a committer doesn't have confidence in a change introduced
>>by the patch, then it should *not* be committed.
> 
> I don't think that's correct. A patch is committed when at one committer
> has the confidence in it, other committer's confidence levels don't
> matter. If others believe the patch should not go in, then they can veto.

I should have said this, which is what I really meant:

A simple definition could be anything that would prevent it from being
committed. If a committer doesn't have confidence in a change introduced
by the patch, then he or she should *not* commit it.

That doesn't preclude the possibility that another committer might veto
the commit.

In other words, as a reviewer state how much you care that a particular
item be fixed.

>>To take a concrete example .....
>>
>>I won't commit a doc patch unless:
>>
>> - Somebody indicates the changes are technically ok.
>> - The DITA doc build succeeds.
>  
> But my (or someone else's) confidence level *might* be lower, if the doc
> builds then it's ok, better to get more eyes on it earlier, thus I
> commit it, even if you don't have confidence.

That's fine -- I view it as an individual committer definition. *I*
(Jean) won't commit a doc patch unless ... [insert the bullets from the
above].  I don't expect others to adopt my criteria.

That much said, I'll be kind of annoyed if somebody does a commit and
the doc build fails.  :-) And only because I know how time consuming it
is to chase down doc build problems.

 -jean

>>But if something isn't worded quite the way I would personally word it,
>>I leave it be -- there's lots of room for style in documentation. Small
>>improvements that advance the info further are good and others can take
>>it even further.
> 
> 
> Right.
> 
> Dan.
> 

Re: New contributors?

[Daniel John Debrunner <dj...@apache.org>]

Jean T. Anderson wrote:

> Daniel John Debrunner wrote:
> 
>>Jean T. Anderson wrote:
>>
>>
>>>It might be helpful for reviewers to clearly separate "must fix" items
>>
>>>from "nice to have" items in their comments.
>>
>>Sounds great, but what is a must fix? 
> 
> 
> A simple definition could be anything that would prevent it from being
> committed. If a committer doesn't have confidence in a change introduced
> by the patch, then it should *not* be committed.

I don't think that's correct. A patch is committed when at one committer
has the confidence in it, other committer's confidence levels don't
matter. If others believe the patch should not go in, then they can veto.

> To take a concrete example .....
> 
> I won't commit a doc patch unless:
> 
>  - Somebody indicates the changes are technically ok.
>  - The DITA doc build succeeds.

But my (or someone else's) confidence level *might* be lower, if the doc
builds then it's ok, better to get more eyes on it earlier, thus I
commit it, even if you don't have confidence.

> 
> But if something isn't worded quite the way I would personally word it,
> I leave it be -- there's lots of room for style in documentation. Small
> improvements that advance the info further are good and others can take
> it even further.

Right.

Dan.

Re: New contributors?

["Jean T. Anderson" <jt...@bristowhill.com>]

Daniel John Debrunner wrote:
> Jean T. Anderson wrote:
> 
>>It might be helpful for reviewers to clearly separate "must fix" items
>>from "nice to have" items in their comments.
> 
> Sounds great, but what is a must fix? 

A simple definition could be anything that would prevent it from being
committed. If a committer doesn't have confidence in a change introduced
by the patch, then it should *not* be committed.

To take a concrete example .....

I won't commit a doc patch unless:

 - Somebody indicates the changes are technically ok.
 - The DITA doc build succeeds.

But if something isn't worded quite the way I would personally word it,
I leave it be -- there's lots of room for style in documentation. Small
improvements that advance the info further are good and others can take
it even further.

 -jean

Re: New contributors?

[Daniel John Debrunner <dj...@apache.org>]

Jean T. Anderson wrote:

> Daniel John Debrunner wrote:
> 
>>Susan L. Cline wrote:

> 
> It might be helpful for reviewers to clearly separate "must fix" items
> from "nice to have" items in their comments.

Sounds great, but what is a must fix? I think most reviewer's comments
are "nice to have". Reviewers don't have any power to say "you must fix
it", unless:
    1) They place a valid technical veto (-1 vote) against the patch. I
think putting -1 against patches as the common practice is not a good
direction, even less friendly.
    2) It's some requirement of the ASF, like no copyright statement in
the code.

I think reviewer's comments are more of the form, "*I* believe the patch
would be better if this was done", and maybe for a committer "*I'm* more
likely to commit the patch if this was done". Maybe we need to make that
clearer. Call them "patch suggestions"?

>>A contributor should be free to challenge or discuss any
>>comment, rather than assume the reviewer is correct. On some of the
>>style issues I raised, related to clarity, I did ask the question "do
>>they add value?". By "add value", I really meant does it increase the
>>clarity of the code? In such cases a contributor is free to come back
>>and say in their opinion it does and why, and maybe the reviewer and
>>community learns something as well, it's a two way street.
> 
> 
> If we want to encourage new contributors to contribute to Derby, we need
> to make sure that the two way street doesn't involving throwing down the
> gauntlet to a new contributor -- otherwise, we'll simply chase new
> contributors away.
> 
> The need to defend a patch can be quite a surprise to new contributors
> who might think that the several days or hours they spend producing
> something will be appreciated, but find they need to pour extra time
> into defending it. 

Not sure "defending" is how we should be putting it. It's more that I
expect contributors to be willing to accept and discuss
comments/suggestions for their patch. It's also about not having
contributors blindly accept that reviewers are correct, it's a peer
based system, new ideas and viewpoints are just as valuable as those of
the existing community members.

We could take the approach that all patches are accepted in any form,
and just hope that contributors turn into community members who care
about "consistently high quality software" rather than continuing on as
contributors who find Derby a great place they can dump any patch in any
state, and someone else will clean it up. This is a valid open source
model. My belief is that it's actually more efficient for the community
if one can address issues at the initial stages rather than once a patch
has landed in the code line and been established for a while. I've spent
too much time cleaning up code in Derby to think any other way :-(

I think it would be great to lower the barrier to get patches into
Derby, but I don't think it should be so low that we are afraid to
provide any feedback to a new contributor for fear of scaring them off.
I hope that at least the contributors get to know the community a little
before diving in, thus have the correct expecations of how things work
and what is required.

One other concern is that a lot of code development is done by copying
existing code, thus non-ideal code has the ability to multiply, almost
by itself. :-) So code without comments leads to more code without
comments, leading to a harder to understand code base leading to less
new contributors getting involved.

[snip]
> If we want new Derby contributors, I think we should do everything we
> can to encourage and increase the confidence of new contributors. Yes,
> there will be items to fix in patches. Let's make an extra effort to be
> clear about what's required and what's nice to have.
> 
> 
>>It's also valid for the contributor to say, those are great ideas, I
>>don't have time to address them now. Then it's up to the committers to
>>decide if one of them has the confidence in the patch.
> 
> 
> I would encourage reviewers to be proactive with new contributors -- for
> example, in your "nice to have" items go ahead and mention that those
> don't have to be done before the patch is committed if the contributor
> doesn't have time. Don't assume that the new contributor is fully versed
> in life at Apache. Make this a confidence-building opportunity.

And it's a two way street, I would encourage new contributors to be
proactive with the community. Discuss items with the community before
working on something rather than being surprised when it's not
immediately accepted by the community.

http://wiki.apache.org/db-derby/PatchAdvice

Dan.

Re: New contributors?

["Jean T. Anderson" <jt...@bristowhill.com>]

Daniel John Debrunner wrote:
> Susan L. Cline wrote:
...
>>This is a somewhat rhetorical question, but should it be the responsibility of the 
>>contributor to address all the comments of different committers? I would imagine this
>>boils down to the desire the contributor has to getting his or her patch committed,
>>which again, I see as a barrier to contribution.
> 
> The contributor is in the best position to address comments from
> reviewers, thus it makes most sense for them to do it. However, the
> comments should be seen as that, not "action items" that are required
> for a patch. 

It might be helpful for reviewers to clearly separate "must fix" items
from "nice to have" items in their comments.

> A contributor should be free to challenge or discuss any
> comment, rather than assume the reviewer is correct. On some of the
> style issues I raised, related to clarity, I did ask the question "do
> they add value?". By "add value", I really meant does it increase the
> clarity of the code? In such cases a contributor is free to come back
> and say in their opinion it does and why, and maybe the reviewer and
> community learns something as well, it's a two way street.

If we want to encourage new contributors to contribute to Derby, we need
to make sure that the two way street doesn't involving throwing down the
gauntlet to a new contributor -- otherwise, we'll simply chase new
contributors away.

The need to defend a patch can be quite a surprise to new contributors
who might think that the several days or hours they spend producing
something will be appreciated, but find they need to pour extra time
into defending it. (I'm thinking of other contacts I've had at Apache on
this topic, not necessarily the specific feedback Susan gave.) I think
we can help by separating the "must fix" items from the "nice to have"
items so the feedback doesn't become overwhelming.

Getting the patch actually committed can be its own gauntlet. This is
getting beyond the intent of Susan's original post, but even posting
that patch to begin with can be intimidating. I have in mind a post to
women@ back in May in which the poster wrote [1]:

| The barrier to entry, at least for the project I know, is
unnecessarily high. A
| meritocracy should reward merit - valuable contributions.
Unfortunately, at
| present, it's a merit-and-confidence-ocracy - valuable contributions
just won't
| be seen, if people don't have the confidence to post them in the first
place,
| and continually plug themselves thereafter.

If we want new Derby contributors, I think we should do everything we
can to encourage and increase the confidence of new contributors. Yes,
there will be items to fix in patches. Let's make an extra effort to be
clear about what's required and what's nice to have.

> It's also valid for the contributor to say, those are great ideas, I
> don't have time to address them now. Then it's up to the committers to
> decide if one of them has the confidence in the patch.

I would encourage reviewers to be proactive with new contributors -- for
example, in your "nice to have" items go ahead and mention that those
don't have to be done before the patch is committed if the contributor
doesn't have time. Don't assume that the new contributor is fully versed
in life at Apache. Make this a confidence-building opportunity.

> Another interesting point is that any reviewer or committer looking at
> the patch do not own it in any way, while some discussion is going on
> between a reviewers and or committers and a contributor about a patch,
> it's fine to another committer to say enough and just commit it.

And it's also fine for somebody else to improve on that work later. The
original contributor doesn't have to incorporate all the "nice to have"
items. Maybe somebody else (the reviewer?) will have the itch to do some
of those.

> Interesting comment about "barrier to contribution" and "desire to get a
> a patch committed". To grow a community we need contributors that are
> willing to follow the Apache way, work with the community to a common
> consensus, thus resolve and discuss reviewers' comments. There's also a
> place for "hit & run" contributors, who provide patches in any state and
> disappear, but those contributors are not going to grow the community.

So here are the 6 principles of the Apache Way drawn out by
http://www.apache.org/foundation/how-it-works.html#management :

    * collaborative software development
    * commercial-friendly standard license
    * consistently high quality software
    * respectful, honest, technical-based interaction
    * faithful implementation of standards
    * security as a mandatory feature

I would include the need to defend a patch as part of that "respectful,
honest, technical-based interaction", but this process can become quite
aggressive at Apache and personally I don't believe that aggression is
part of the "Apache Way" (actually, I think it *was* part of the
original Apaches). I think it's just behavior that a lot of projects
exhibit for whatever reason, mostly social and demographic, and often
because the volunteers reviewing and committing patches have a finite
amount of time.

> Thanks for bringing this issue out for discussion,

indeed!

 -jean

[1]
http://mail-archives.apache.org/mod_mbox/www-women/200605.mbox/%3c20060521225909.GA20525@dochas.stdlib.net%3e

Re: New contributors?

[Daniel John Debrunner <dj...@apache.org>]

Susan L. Cline wrote:

[snip feedback on style discussion]
> 
> I felt discouraged because I think if it is a style issue or it is a nit, then why
> does it need to be mentioned?  I'm not sure if others feel this way, but I can say
> personally, when submitting a patch (which I mentioned I have rarely done) I already feel
> a little bit nervous about it being coded correctly, without regard to style.

I think some of this depends on each person's definition of a "style
issue". Most of what might be called style issues I raise I believe have
to do with clarity of the resulting code. I believe this is important
for the long term clarity of the entire code base which I also believe
lowers the barrier for entry for new contributors. So I'm willing to not
 commit a technically correct patch that's full of confusing code or
lack of comments because it will lead to a harder to understand code
base that in the long run will be worse for the community. So I see that
as a trade-off between the short-term of making it easier for one
individual to get involved against making the long term easier for
multiple people to get involved.

As specific examples of issues that some may see as style, and I as clarity:

  - Logical naming of items (variables, fields, methods, classes etc.) I
really believe that this makes code much easier to read and understand,
especially for newcomers. Two examples are:
       *  when using a specific name would be clearer 'lengthInBytes'
rather than 'len' (e.g. is that length in characters or bytes?)
       * ensuring the name correctly refects the object's purpose. As a
crude example:

          public Time getBreakfastTime() {
               return lunchTime;
          }
        People reading code that calls this method will see
'.getBreakfastTime()', thus assume the time being fetched is breakfast
time, thus leading to wrong assumptions and bugs.


   - Comments for methods and fields. Without comments it becomes really
hard for a reviewer or anyone reading the committed code to understand
the code, the problem really is that there is no guidance as to what the
contributor *intended* the functionality to be. The Java code in a
method may look ok, but is it what was intended or required? The human
readable comment describing the functionality (or intended purpose for a
field) really ties the intended behaviour to the code, and gives more
opportunity to discover bugs.


Maybe there's also an issue in the way we describe comment items: "nits"
&  "style issue", Kristian said item number 2) was a "nit", but look at
his comment "I had to spend a little time thinking about why you would
need that, until PI popped up in my head :) " - A comment to the method
would have meant he (and countless others) understood it without
spending time. Is that really a "nit", or is it useful feedback?

[snip - more background]

>>Since Apache is a peer based community, maybe it should be made clearer
>>that reviewers' comments are just "their comments", the contributor
>>doesn't have to follow all, or even any, of the advice. Any committer
>>can commit any patch that they "have a high degree of confidence in".
>>
>>In my opinion a committer may gain confidence in a patch by requesting
>>certain changes in it through review comments. If those changes don't
>>happen or partially happen, then committer can re-evaluate their
>>confidence level in the patch, possibly leading to committing the patch.
>>If the committer decides not to commit the patch, then it's possible
>>they may modify it to have confidence in it and then submit it, though
>>this might occur much later when the committer (or anyone else) has time.
> 
> 
> This makes sense to me, and in a way is part of the reason I am confused
> by the process.  After your gave me your comments I believe I addressed all of
> them, even the "style" comments.  However, then I received a second set
> of comments from another person which were different from yours. 

Different, but not contradictory. I think that's a fact of open source,
or any code reviewing, different people see different things, have
different hot buttons based upon past experience. It's also the value of
the "multiple eyes".

> This is a somewhat rhetorical question, but should it be the responsibility of the 
> contributor to address all the comments of different committers? I would imagine this
> boils down to the desire the contributor has to getting his or her patch committed,
> which again, I see as a barrier to contribution.

The contributor is in the best position to address comments from
reviewers, thus it makes most sense for them to do it. However, the
comments should be seen as that, not "action items" that are required
for a patch. A contributor should be free to challenge or discuss any
comment, rather than assume the reviewer is correct. On some of the
style issues I raised, related to clarity, I did ask the question "do
they add value?". By "add value", I really meant does it increase the
clarity of the code? In such cases a contributor is free to come back
and say in their opinion it does and why, and maybe the reviewer and
community learns something as well, it's a two way street.
It's also valid for the contributor to say, those are great ideas, I
don't have time to address them now. Then it's up to the committers to
decide if one of them has the confidence in the patch.
Another interesting point is that any reviewer or committer looking at
the patch do not own it in any way, while some discussion is going on
between a reviewers and or committers and a contributor about a patch,
it's fine to another committer to say enough and just commit it.

Interesting comment about "barrier to contribution" and "desire to get a
a patch committed". To grow a community we need contributors that are
willing to follow the Apache way, work with the community to a common
consensus, thus resolve and discuss reviewers' comments. There's also a
place for "hit & run" contributors, who provide patches in any state and
disappear, but those contributors are not going to grow the community.

Thanks for bringing this issue out for discussion,
Dan.

Re: New contributors?

["Susan L. Cline" <ho...@pacbell.net>]

--- Daniel John Debrunner <dj...@apache.org> wrote:

> Susan L. Cline wrote:
> 
> > Hello Derby-dev, 
> > 
> > I'm writing to point out an experience which discouraged me about growing our community with
> new
> > contributors. 
> > 
> > Recently I submitted a patch for a JUnit test.   I have contributed very few patches in the
> form
> > of code to Derby (I've had just a couple for the Eclipse plug-ins.)  I am not a Java developer
> by
> > trade or training, but I have contributed to the Derby community by writing 
> > articles, sample applications and some wiki entries. 
> > 
> > My JUnit contribution received comments by two different folks.  I believe that some of the
> > comments were valid and helpful to me to improve my programming skills and also to help insure
> > that the test I wrote was readable and understandable, as well as to fit into the constraints
> that
> > JUnit imposes as well as the Derby test harness. 
> > 
> > However, I feel that some of the comments were more style-related or were nits that the
> individual
> > reviewer had.  This is where I feel discouraged. 
> 
> As one of the reviewers I'm sorry you felt discouraged, that was never
> my intention and I'm sure it wasn't Kristian's (as the other reviewer).
> 
> Looking back at the set of comments I can see comments about various
> style issues, but to my reading none of them seem to say "you must do it
> my way", in fact they include phrases like "Maybe a style issue, but I
> prefer not to have constants like", and "2 and 3 are nits, use your own
> judgement. ". I personally could use help to know exactly what
> discouraged you, so that any review comments in the future would not
> discourage contributors.
> 

I felt discouraged because I think if it is a style issue or it is a nit, then why
does it need to be mentioned?  I'm not sure if others feel this way, but I can say
personally, when submitting a patch (which I mentioned I have rarely done) I already feel
a little bit nervous about it being coded correctly, without regard to style.

Maybe another example in an area I feel more confident in will help to illustrate the point.
If someone takes the time to contribute to the Derby wiki, or to write an article about Derby
I would hope the community would provide feedback about the technical accuracy of the article
without regard to a specific style.  We all have different styles - for someone who writes 
frequently and fluently that style may be seen as more preferred, but for someone who does not
trying to achieve this same style may be forced and artifical.
I would hope that we as a community would allow these differences.

I'm not sure if I am making my point more clearly to help you understand, but I guess it is
an attempt at explaining if you feel like you are contributing something and you are already
worried about the technical accuracy, you may not be feel very encouraged when given feedback 
about the style of something.

> > One of the biggest goals I think we have is to
> > embrace contributors (and therefore their contributions) and to guide them 
> > where it is necessary, but also to accept contributions that represent "incremental work." 
> Also,
> > I think when someone makes a contribution it has something to do with that individuals "itch".
>  In
> > this case I thought it would be fun to learn JUnit and become more familiar with the 
> > Derby test harness, and that was my itch.
> > 
> > I am fortunate in that I am paid to work on Derby, but I think of other individuals who are
> not
> > paid to work on Derby.  If they make a contribution which is not accepted, partially due to
> style
> > issues, how likely are we to get new contributors?   Again, I'm not saying that I wrote the
> > perfect test and others are not welcome to comment on my code, but on the other hand if we
> make
> > the barrier to contribute (and it was just a test!) this high, how likely are we to get
> > contributions from individuals who have a lesser stake in Derby than their own livelihood? 
> 
> I agree it would be very bad if the Derby community did not accept
> patches that practiced incremental development, or based upon style
> issues. I don't think that has ever happened.
> 
> I guess I'm a little confused by your last two paragraphs, the community
> should "guide them" in the first paragraph, which is what I thought the
> review comments were, but in the second paragraph it seems you view the
> same review comments as a barrier.

I believe I differentiated between types of comments.  Those that are based on
technical issues and comments and those that are style or "nit" comments.  I believe
that the community should guide them via the technical comments, but if the comments
become based upon style or personal nits then yes, they are a barrier to submission,
or at least to iteratively submitting a patch.  

> 
> Since Apache is a peer based community, maybe it should be made clearer
> that reviewers' comments are just "their comments", the contributor
> doesn't have to follow all, or even any, of the advice. Any committer
> can commit any patch that they "have a high degree of confidence in".
> 
> In my opinion a committer may gain confidence in a patch by requesting
> certain changes in it through review comments. If those changes don't
> happen or partially happen, then committer can re-evaluate their
> confidence level in the patch, possibly leading to committing the patch.
> If the committer decides not to commit the patch, then it's possible
> they may modify it to have confidence in it and then submit it, though
> this might occur much later when the committer (or anyone else) has time.

This makes sense to me, and in a way is part of the reason I am confused
by the process.  After your gave me your comments I believe I addressed all of
them, even the "style" comments.  However, then I received a second set
of comments from another person which were different from yours. 
 
This is a somewhat rhetorical question, but should it be the responsibility of the 
contributor to address all the comments of different committers? I would imagine this
boils down to the desire the contributor has to getting his or her patch committed,
which again, I see as a barrier to contribution.

 
> Thanks,
> Dan.
> 
> 
> 

Re: New contributors?

[Daniel John Debrunner <dj...@apache.org>]

Susan L. Cline wrote:

> Hello Derby-dev, 
> 
> I'm writing to point out an experience which discouraged me about growing our community with new
> contributors. 
> 
> Recently I submitted a patch for a JUnit test.   I have contributed very few patches in the form
> of code to Derby (I've had just a couple for the Eclipse plug-ins.)  I am not a Java developer by
> trade or training, but I have contributed to the Derby community by writing 
> articles, sample applications and some wiki entries. 
> 
> My JUnit contribution received comments by two different folks.  I believe that some of the
> comments were valid and helpful to me to improve my programming skills and also to help insure
> that the test I wrote was readable and understandable, as well as to fit into the constraints that
> JUnit imposes as well as the Derby test harness. 
> 
> However, I feel that some of the comments were more style-related or were nits that the individual
> reviewer had.  This is where I feel discouraged. 

As one of the reviewers I'm sorry you felt discouraged, that was never
my intention and I'm sure it wasn't Kristian's (as the other reviewer).

Looking back at the set of comments I can see comments about various
style issues, but to my reading none of them seem to say "you must do it
my way", in fact they include phrases like "Maybe a style issue, but I
prefer not to have constants like", and "2 and 3 are nits, use your own
judgement. ". I personally could use help to know exactly what
discouraged you, so that any review comments in the future would not
discourage contributors.

> One of the biggest goals I think we have is to
> embrace contributors (and therefore their contributions) and to guide them 
> where it is necessary, but also to accept contributions that represent "incremental work."  Also,
> I think when someone makes a contribution it has something to do with that individuals "itch".  In
> this case I thought it would be fun to learn JUnit and become more familiar with the 
> Derby test harness, and that was my itch.
> 
> I am fortunate in that I am paid to work on Derby, but I think of other individuals who are not
> paid to work on Derby.  If they make a contribution which is not accepted, partially due to style
> issues, how likely are we to get new contributors?   Again, I'm not saying that I wrote the
> perfect test and others are not welcome to comment on my code, but on the other hand if we make
> the barrier to contribute (and it was just a test!) this high, how likely are we to get
> contributions from individuals who have a lesser stake in Derby than their own livelihood? 

I agree it would be very bad if the Derby community did not accept
patches that practiced incremental development, or based upon style
issues. I don't think that has ever happened.

I guess I'm a little confused by your last two paragraphs, the community
should "guide them" in the first paragraph, which is what I thought the
review comments were, but in the second paragraph it seems you view the
same review comments as a barrier.

Since Apache is a peer based community, maybe it should be made clearer
that reviewers' comments are just "their comments", the contributor
doesn't have to follow all, or even any, of the advice. Any committer
can commit any patch that they "have a high degree of confidence in".

In my opinion a committer may gain confidence in a patch by requesting
certain changes in it through review comments. If those changes don't
happen or partially happen, then committer can re-evaluate their
confidence level in the patch, possibly leading to committing the patch.
If the committer decides not to commit the patch, then it's possible
they may modify it to have confidence in it and then submit it, though
this might occur much later when the committer (or anyone else) has time.

Thanks,
Dan.