Hey,

I know we talked offline about most of this, but I'll leave things
here for recording purposes.

On Mon, Mar 16, 2009 at 9:39 AM, Brian Granger <email address hidden> wrote:
> Review: Needs Fixing
> I just had fun doing this code review.  Launchpad has really improved its code review interface.  I really like how you can look through the diffs for each file now!


Yes, it's gotten much better.  Launchpad is slowly but surely improving.

>
> One general comment:  I know you like to code in a single branch, but it makes it more difficult to review a branch when there are many revisions on a wide range of topics.  I don't think each revision should go in its own branch, but when there is a larger set of revisions addressing a single issue, having all of those revisions in a single topic branch would be nice and make review easier.  Also, smaller branches allow code that *is* finished to get into trunk faster without waiting for unrelated code to be finished and reviewed.  I am guilty of the "big branch" workflow myself though.

Me too, it's certainly a bad habit.  As we spoke about, we could
probably settle on a workflow with:

- per-person permanent 'trunk-dev' branches, on which we do easy,
incremental reviewing and merging.
- short-lived 'feature branches' for standalone ideas, more like a
patch in other workflows, just a patch that keeps evolving and that's
easy to track as it changes.

I'll try to do better in the future on this, and I really appreciate
that you took the time to go over this much code.  Many thanks.


>
> r1168:
>
> As you go through file and clean up the copyright info, please convert the copyright header to our standard one that references the ipython development team. Any specific mention of module authors or maintainers should go into the module docstring.

As I mentioned, I was really going after the 'import Release'
statements, not after the authorship notices.   But from now on, I'll
try to cleanup the copyright notices as I work on files.

> r1169:
>
> Are the numpy helper scripts for converting docstrings in place and working with our IPython?
>
> How do we want to handle references in our docs.  Sphinx can now create a nice references section in the pdf files, but you have to use a particular format for the references.  The current form of the references you have put in::
>
>    .. _PEP 257: http://www.python.org/peps/pep-0257.html
>
> I don't think will end up in the references section.  How does numpy handle such things.  Have a look at how references are handled in the kernel documentation to see how I have been handling this.  My way may not be the best.  Let's figure out how all of us should handle this and then document it in our docs.

This one, I don't know. I'm going to look into what Matthew Brett did
for Nipy to see if I can find a good overall solution.

> r1171:
>
> In ipy_profile_zope.py.  Should we just remove all the old __author__, etc. module attributes?

I'll do what we discussed: put the authorship notes I find in the
top-level docstring.  This has the added benefit of putting them in
the generated API docs.

> All other revisions are great.

Thanks again for reviewing such a monster!

I'll work on the notes above and will post again for final review.

Cheers,

f