Zulip Chat Archive

Stream: PR reviews

Topic: 4447 migrate to dense API


view this post on Zulip 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.

view this post on Zulip Eric Wieser (Oct 13 2020 at 16:06):

#4447

view this post on Zulip Patrick Massot (Oct 13 2020 at 16:22):

Maybe we're too formal here with the review process and we should simply merge it.

view this post on Zulip Heather Macbeth (Oct 13 2020 at 16:36):

I'll take a look, tonight if not sooner. Sorry for the delay!

view this post on Zulip 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.

view this post on Zulip Heather Macbeth (Oct 13 2020 at 19:33):

No, please, go ahead!

view this post on Zulip Sebastien Gouezel (Oct 13 2020 at 19:53):

Done!

view this post on Zulip 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.

view this post on Zulip 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: May 06 2021 at 13:21 UTC