Code review comment for lp:~ipython-dev/ipython/kernel-config

Revision history for this message
Brian Granger (ellisonbg) wrote :

> I'm not merging it now just in case you want to do it yourself tomorrow (12/30) first thing or have some other little change you'd like to put in.  But if you don't merge it I'll go ahead and do it, and will begin adding code I have (like testing stuff) so we can push towards 0.11.  We still have cleanups and things to do, but all of that is best done on top of a more functional trunk, which this branch helps LOT with.

Thanks for the review. Go ahead and do the merge, I am working on
other things right now.

> Per-commit comments:
>
> -r1264: ok
>
> -r1265: in kernel/ipclusterapp.py, should we output something visible to the user indicating the 4s pause at shutdown?  I won't add it myself, as I'm not sure this should go in; most of our output is now via logging, right?  Putting this in the log wouldn't be too useful, it would be strictly a convenience for the user.

Yes, I can add a message about this to the logs.

> -r1266: ok, just upstream
>
> -r1267: great, more examples!
>
> -r1268: good, these are changes we'd discussed over the phone.
>
> -r1269: better docs, excellent...
>
> -r1270, 71, 72, 73: this document is stellar, we'd reviewed it already.
>
> --
> https://code.launchpad.net/~ipython-dev/ipython/kernel-config/+merge/14900
> You proposed lp:~ipython-dev/ipython/kernel-config for merging.
>

--
Brian E. Granger, Ph.D.
Assistant Professor of Physics
Cal Poly State University, San Luis Obispo
<email address hidden>
<email address hidden>

« Back to merge proposal