On Fri, Mar 9, 2018 at 11:51 AM, Stefan van der Walt <stefanv@berkeley.edu> wrote:
On Fri, 09 Mar 2018 17:00:43 +0000, Stephan Hoyer wrote:
I'll note that we basically used GitHub for revising __array_ufunc__ NEP, and I think that worked out better for everyone involved. The discussion was a little too specialized and high volume to be well handled on the mailing list.
A disadvantage of GitHub PR comments is that they do not track sub-threads of conversation, so you cannot "reply to" a previous concern directly.
Yeah, I actually find email much easier for this kind of complex high-volume discussion. Even if lots of people don't use traditional threaded mail clients anymore [1], archives are still threaded, and the tools that make line-by-line responses easy and the ability to split off conversations are both really helpful. (E.g., the way I split this thread off from the original one :-).) The __array_ufunc__ discussion was almost impenetrable on GH, I think. I admit though that some of this is probably just that I'm more used to the email-based discussion workflow. Honestly none of these tools are particularly amazing, and the __array_ufunc__ conversation would have been difficult and inaccessible to outsiders no matter what medium we used. It's much more important that we just pick something and use it consistently than that pick the Most Optimal Solution. [1] Meaning this, not gmail's threads: https://en.wikipedia.org/wiki/Conversation_threading#/media/File:Nntp.jpg
PRs also mix inline comments (that become much less visible after rebases and updates) and "story line" comments. These two "modes" of commenting, substantive discussion around ideas, v.s. concerns about specific phrasing, usage of words, typos, content of code snippets, etc., may require different approaches. It would be quite easy to redirect the prior to the mailing list and the latter to the GitHub PR.
I don't think we should worry about this. Fiddly detail comments are, by definition, not super important, and generally make up a tiny volume of the discussion around a proposal. Also in practice reviewers are no good at splitting up substantive comments from fiddly details: the review workflow is that you read through and as thoughts occur you write them down, so even if you start out thinking "okay, I'm only going to comment on typos", then half-way through some paragraph sparks a thought and suddenly you're writing something substantive (and I'm as guilty of this as anyone, maybe more so...). Asking people to classify their comments and then chiding them for putting them in the wrong place etc. isn't a good use of time. Let's just pick one place for everything and stick with it.
I'm also not too keen on repeated PR creation and merging (it splits up the PR discussion even further). Why not simply hold off until the PEP is ready, and view the documents on GitHub? The rendering there is just as good.
Well, if we aren't using PRs for discussion then multiple PRs are fine :-). And merging changes quickly is helpful because it makes the rendered NEPs page a single one-stop-shop to see all the latest NEPs, no matter what their current status. If we do use PRs for discussion, then I agree that we should try to keep the PR open until the NEP is "done", to minimize the splitting of discussion. This does create a bit of extra friction because it turns out that "is this done?" is not something you can really ever answer for certain :-). Even after PEPs are accepted they usually end up getting some further tweaks once people start implementing them. Sometimes PEPs get abandoned in "Draft" state without ever being accepted/rejected, and sometimes a PEP that had been abandoned for years gets picked up and finished. You can see this in the Rust RFC guidelines too [2]; they specifically address the issue of post-merge changes, and it sounds like their solution is that if a substantive issue is discovered in an accepted RFC, then you have to create a new "fixup" RFC, which then gets its own PR for discussion. I guess if this were our process then __array_ufunc__ would have ended up with ~3 NEPs :-). This is all doable -- every approach has trade-offs. But we should pick one, so we can adapt to those trade-offs. [2] https://github.com/rust-lang/rfcs#the-rfc-life-cycle -n -- Nathaniel J. Smith -- https://vorpus.org