[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