[SciPy-dev] Scipy workflow (and not tools).

Travis E. Oliphant oliphant@enthought....
Wed Feb 25 11:41:52 CST 2009


Stéfan van der Walt wrote:
> Cha
> Thank you for providing some prime examples on why code review and
> testing is needed.
>
> I didn't know about the changes to the optimisation module, but now I
> have to ask these questions:
>
> 1. Is it quality code, suitable for SciPy?  (It is, I read the code,
> or in other words *reviewed* it)
>
> 2. Does it work?  I don't know.  Nobody does.  There aren't any tests,
> no guarantees.
>   
Which code has no tests?   Are you speaking rhetorically or about a 
specific check-in?
> That would be my reaction to one change, but there were 6 or more,
> none with any tests (and at least one contains a spelling mistake!).
> How do we know that they work under Windows, Solaris, Linux and OSX?
>
> Worse; what if I decide to make some updates to that code but, not
> having understood the author's intention perfectly, break it horribly.
>  Who would be any the wiser?
>
> Tests protect the user and the developer alike.  It is irresponsible
> to carry on the way we do.
>   
No it's not.   It's just a different, more organic, development model 
than the one you are championing.   It's one where people do their best 
to create quality code and help fix the pieces that they care about, no 
matter who the original author was.

Getting quality code is not as easy as establishing a formal review 
process.   We've had a review process for years.   *Everybody* is 
welcome to review the code that is checked in and complain / argue about 
better ways to do it.   If you feel that strongly and have commit 
access, you can even change the code.
>> Are we adding a lot of broken code?
>>     
>
> Yes, we are.  
There are *degrees* of broken-ness.  It's impossible to prove that code 
is not "broken" in some way depending on your definition of broken.   
So, it's a bit misleading to complain about broken-ness.  

We are also adding a lot of very useful code.   I agree code gets better 
with more eye-balls, generally, but developers are also not 
interchangeable.  I'm  not at all convinced that a formal review process 
will actually make things better, when what we need is more time spent. 

> More folks with commit priviledges would just perpetuate this chaos.
> Our community is sophisticated enough not to apply a brute-force
> solution to the problem.
>   
I'm not sure it actually would.    I can see, however, that we do need a 
DVCS system to make the work-flow easier because branching is the key to 
having real development take place and allow the developer to utilize 
the benefits of version control while not impacting the "released-line" 
until they are ready to commit all changes (i.e. branching and merging 
needs to be as seamless as possible).   
> It should not be anyone's job to clean up after his/her peers.  If
> each patch is accompanied by appropriate tests, this situation would
> never occur.
>   
I think this is a fundamentally wrong mindset.   Yes, we should all test 
before we commit code, and I'm not opposed to reminding people that the 
tests they already use to make sure things work can be easily placed as 
unit-tests.   

But, my best effort in an area which actually solves the problem, or 
scratches-the-itch I had, may look like "a mess" to someone else.   And 
I don't think we can find a one-size fits-all solution to that 
fundamental difference of perspective.   Who decides what "clean-up" 
constitutes?   My answer is "those most interested" which is a dynamic 
and possibly varying group.

If there are concerns about the commits people are making, then lets 
talk about specifics and address those rather than masking that behind a 
"review process"  

-Travis



More information about the Scipy-dev mailing list