[SciPy-Dev] pull requests and code review

Travis Oliphant travis@continuum...
Thu Jan 5 19:02:03 CST 2012


Wow that is a much different time frame than I would expect or think necessary.

Where does this rule of thumb come from?

It has quite a few implications that I don't think are pleasant.  I would be quite dissatisfied if my pull requests took 2-3 weeks to get merged.

Is that just your feeling or does that come from empirical data or a larger vote?  

-Travis 

--
Travis Oliphant
(on a mobile)
512-826-7480


On Jan 5, 2012, at 6:53 PM, Charles R Harris <charlesr.harris@gmail.com> wrote:

> 
> 
> On Thu, Jan 5, 2012 at 5:41 PM, Travis Oliphant <travis@continuum.io> wrote:
> I don't think there should be any time limit on pull requests.     Who is the *we* that needs time to look at them.   I did take time to look at the changes.   The code changes were not extensive (except for the very nice tests), and it is a welcome change.
> 
> Your feedback on the use of inspect is very good.   We can take a look at whether or not it method calls were considered and fix it if it does.
> 
> If you are interested in continuing to review all the optimize changes, I will make sure and give you time to review in the future.    This is where having a list of interested and available parties for different modules would make a great deal of sense.
> 
> 
> The rule of thumb would be two days for two or three line fixups, a week or more for more intrusive stuff together with a note to the list. If you need the code *right now*, then keep developing off line until the whole package is ready. You'd be surprised at the feedback even "trivial" things can elicit.
> 
> Chuck 
> 
> _______________________________________________
> SciPy-Dev mailing list
> SciPy-Dev@scipy.org
> http://mail.scipy.org/mailman/listinfo/scipy-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.scipy.org/pipermail/scipy-dev/attachments/20120105/6883556a/attachment.html 


More information about the SciPy-Dev mailing list