Re: [openstack-dev] Re : welcoming new committers (was Re: When is it okay for submitters to say 'I don't want to add tests' ?)

2013-11-04 Thread John Dennis
On 10/31/2013 10:36 PM, Jeremy Stanley wrote:
 As has been said many times already, OpenStack does not lack
 developers... it lacks reviewers.

In regards to reviews in general and in particular for welcoming new
committers I think we need to be careful about reviewers NAK'ing a
submission for what is essentially bikeshedding [1]. Reviewers should
focus on code correctness and adherence to required guidelines and not
NAK a submission because the submission offends their personal coding
preferences [2].

If a reviewer thinks the code would be better with changes which do not
affect correctness and are more in the vein of style modifications
they should make helpful suggestions but give the review a 0 instead of
actually NAK'ing the submission. NAK'ed reviews based on style issues
force the submitter to adhere to someone else's unsubstantiated opinion
and slows down the entire contribution process while submissions are
reworked multiple times without any significant technical change. It's
also demoralizing for submitters to have their contributions NAK'ed for
reasons that are issues of opinion only, the submitter has to literally
submit [3].

[1] http://en.wiktionary.org/wiki/bikeshedding

[2] Despite the best attempts of computer science researchers over the
years software development remains more of a craft than a science with
unambiguous rules yielding exactly one solution. Often there are many
valid approaches to solve a particular coding problem, the selection of
one approach often boils down to the personal preferences of the
craftsperson. This does not diminish the value of coding guidelines
gleaned from years of analyzing software issues, what it does mean is
those guidelines still leave plenty of room for different approaches and
no one is the arbiter of the one and only correct way.

[3] to give over or yield to the power or authority of another.

-- 
John

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] Re : welcoming new committers (was Re: When is it okay for submitters to say 'I don't want to add tests' ?)

2013-11-04 Thread Ravi Chunduru
Good points from John.

The only concern for first time reviewers is that their comments gets
overseen by the committer. If the review comment is good, I feel
core-reviewer must put some weight on it and thus encourage genuine
suggestions.


On Mon, Nov 4, 2013 at 9:33 AM, John Dennis jden...@redhat.com wrote:

 On 10/31/2013 10:36 PM, Jeremy Stanley wrote:
  As has been said many times already, OpenStack does not lack
  developers... it lacks reviewers.

 In regards to reviews in general and in particular for welcoming new
 committers I think we need to be careful about reviewers NAK'ing a
 submission for what is essentially bikeshedding [1]. Reviewers should
 focus on code correctness and adherence to required guidelines and not
 NAK a submission because the submission offends their personal coding
 preferences [2].

 If a reviewer thinks the code would be better with changes which do not
 affect correctness and are more in the vein of style modifications
 they should make helpful suggestions but give the review a 0 instead of
 actually NAK'ing the submission. NAK'ed reviews based on style issues
 force the submitter to adhere to someone else's unsubstantiated opinion
 and slows down the entire contribution process while submissions are
 reworked multiple times without any significant technical change. It's
 also demoralizing for submitters to have their contributions NAK'ed for
 reasons that are issues of opinion only, the submitter has to literally
 submit [3].

 [1] http://en.wiktionary.org/wiki/bikeshedding

 [2] Despite the best attempts of computer science researchers over the
 years software development remains more of a craft than a science with
 unambiguous rules yielding exactly one solution. Often there are many
 valid approaches to solve a particular coding problem, the selection of
 one approach often boils down to the personal preferences of the
 craftsperson. This does not diminish the value of coding guidelines
 gleaned from years of analyzing software issues, what it does mean is
 those guidelines still leave plenty of room for different approaches and
 no one is the arbiter of the one and only correct way.

 [3] to give over or yield to the power or authority of another.

 --
 John

 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev




-- 
Ravi
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] Re : welcoming new committers (was Re: When is it okay for submitters to say 'I don't want to add tests' ?)

2013-11-01 Thread David Kranz

On 10/31/2013 10:36 PM, Jeremy Stanley wrote:

On 2013-10-31 22:45:56 + (+), Romain Hardouin wrote:

Adding a message for new comers is a good idea.
I am a new Horizon contributor, some of my fixes have been merged
(thanks to Upstream University :-) and reviewers of course) but I
still hesitate to do code review. To my mind, it is reserved to
known developpers whose opinion matters...

Not at all. One of the best ways to become known within the
community is to review code and provide good recommendations. Even
something as simple as spotting typographical errors in changes to
user-facing messages and documentation provides value. The more
problems you can find (and ultimately help prevent) in a change, the
faster your reputation will grow.

As has been said many times already, OpenStack does not lack
developers... it lacks reviewers.
Reviewing and contributing unit tests are the developer activities we 
have for addressing quality. I think the issue here is how we as a 
community make sure there is balance between these activities and raw 
feature (and bug) contribution, given that most developers most enjoy 
hacking away, myself included. In a corporate software project, this 
balance would be enforced by one or all of:


1. Slowing down development
2. Providing more qa resources, including requiring developers to write 
unit tests
3. Knowingly accepting quality risk in exchange for some 
business-related gain


As an open source community we cannot do some of these things. But lack 
of reviewers effectively slows down development, and we can strive for 
the scalability of quality that comes from developers writing unit 
tests. My first contribution to swift was rejected until I enhanced the 
test infrastructure even though what I did was similar to other things 
that were not really being tested.


We should be nice about it, and spend a little extra effort in helping 
new contributors get into the swing of writing unit tests, but the 
review gate is the only real mechanism we have for making sure we have 
sufficient coverage to keep the code base maintainable by others in the 
future. I really like Rob's list because it leads down a path of better 
shared understanding of how lax/lenient reviewers should be about this.


 -David

 -David

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


[openstack-dev] Re : welcoming new committers (was Re: When is it okay for submitters to say 'I don't want to add tests' ?)

2013-10-31 Thread Romain Hardouin
Hi all, br/br/Adding a message for new comers is a good idea. br/I am a 
new Horizon contributor, some of my fixes have been merged (thanks to Upstream 
University :-) and reviewers of course) but I still hesitate to do code review. 
To my mind, it is reserved to known developpers whose opinion matters... 
br/br/- Romain___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] Re : welcoming new committers (was Re: When is it okay for submitters to say 'I don't want to add tests' ?)

2013-10-31 Thread Jeremy Stanley
On 2013-10-31 22:45:56 + (+), Romain Hardouin wrote:
 Adding a message for new comers is a good idea.
 I am a new Horizon contributor, some of my fixes have been merged
 (thanks to Upstream University :-) and reviewers of course) but I
 still hesitate to do code review. To my mind, it is reserved to
 known developpers whose opinion matters...

Not at all. One of the best ways to become known within the
community is to review code and provide good recommendations. Even
something as simple as spotting typographical errors in changes to
user-facing messages and documentation provides value. The more
problems you can find (and ultimately help prevent) in a change, the
faster your reputation will grow.

As has been said many times already, OpenStack does not lack
developers... it lacks reviewers.
-- 
Jeremy Stanley

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev