Zulip Chat Archive
Stream: PR reviews
Topic: 4447 migrate to dense API
Yury G. Kudryashov (Oct 13 2020 at 16:05):
@Sebastien Gouezel @Heather Macbeth Could you please have a look at this PR? Most of it was already reviewed by @Patrick Massot but he pushed some commits to this branch, so we need someone else to merge it. I'm afraid that the PR will rot.
Eric Wieser (Oct 13 2020 at 16:06):
Patrick Massot (Oct 13 2020 at 16:22):
Maybe we're too formal here with the review process and we should simply merge it.
Heather Macbeth (Oct 13 2020 at 16:36):
I'll take a look, tonight if not sooner. Sorry for the delay!
Sebastien Gouezel (Oct 13 2020 at 19:30):
I had a look. It looks very good to me, I just had two minor docstrings comments. I haven't bors d+ed it since Heather might want to have a look.
Heather Macbeth (Oct 13 2020 at 19:33):
No, please, go ahead!
Sebastien Gouezel (Oct 13 2020 at 19:53):
Done!
Scott Morrison (Oct 15 2020 at 02:42):
Generally, I don't think we need to require that a maintainer pushing a few changes automatically disqualifies them to merge... If the author has responded positively to the changes (particularly if they are also a maintainer!) let's just do it.
Scott Morrison (Oct 15 2020 at 02:42):
Of course it's also fine to excuse oneself, and a good idea whenever you're not entirely sure your changes are actually awesome. :-)
Last updated: Dec 20 2023 at 11:08 UTC