[IPython-dev] QT-console bugfix commits and shepherding

[Fernando Perez]

Hi Jonathan,

On Fri, Apr 13, 2012 at 7:28 AM, Jonathan March <JDM at marchray.net> wrote:
> Hi core folk,
>
> There are two new qtconsole bugfix PRs
> https://github.com/ipython/ipython/pull/1569

There are some minor nits to be resolved here, and then it looks good to go.

> https://github.com/ipython/ipython/pull/1573

Yup, I see you merged it after my feedback, thanks!  BTW, as a
guideline for future merges, in the 'blue box' merge widget that
github offers, try to always add at the end a line that says

Closes #NN...

so that any relevant issues get auto-closed upon merge.  In this case,
I closed manually #1574, but it's a good idea to keep the merge
messages as a summary of the issues the entire merge addresses (esp.
since a large merge may fix multiple issues).

> I with colleagues at Enthought have reviewed them both, and I would like to
> ask permission to merge them.
>
> I have commit rights, which I have not hitherto used for non-trivial
> commits.
>
> Since Enthought is using qtconsole regularly, while you are deeply engaged
> in notebook development, and @epatters is busy with school, I would also
> offer to take on the shepherding and reviewing of qtconsole PRs, if that's
> agreeable to you. I am by no means QT-wise, but can draw on the considerable
> expertise of colleagues.

That would be great.  You should still allow time for other devs to
also have a chance to have a look; e.g. for #1573 I had some small
suggestions for improvements before merging.  But if you guys at
Enthought give it a solid round of testing and review, that
drastically reduces the burden on other reviewers, and the chances of
a fast merge go way up.

So as a guideline, I suggest: you (collective Enthought you)
test/review these Qt-focused PRs (and you're welcome to do the same in
other parts of the codebase, of course), and leave that feedback in
the PR discussion so everyone else can see it.  If in a few days once
it's been reviewed you haven't gotten any other eyes on that
particular PR from anyone else, I suggest you give us one last ping
on-list, and if nobody responds within a reasonable delay (say a day
or two in non-crazy-holiday times), go ahead and merge.  This should
give everyone a chance to have a look at the code, while not
bottlenecking you.

It's fantastic to see that you guys can take some of the effort on
this code, because as you correctly point out, right now we don't have
the bandwidth for it.

How does the above sound?

Cheers,

f

ps - as a quick reminder, when doing merges, even single-commit ones,
let's always use the github merge button, and put in a reasonable
message that describes the overall PR (especially useful for a long,
multi-commit PR).  A good starting point for that message is often the
introductory message of the PR itself, though I often manually tweak
things for clarity and conciseness.