Stream: t-compiler

Topic: design meeting 2019-12-20


nikomatsakis (Dec 20 2019 at 14:18, on Zulip):

Hey @T-compiler/meeting -- design meeting still start here in 40 minutes. As noted in #t-compiler > pre-design meeting 2019-12-19, the plan is to discuss compiler-team#209, "major changes proposal". @mw and I drafted up a hackmd here with some motivations and an initial draft proposal. The high-level summary, though, is as follows:

pnkfelix (Dec 20 2019 at 14:28, on Zulip):

"reviewing with context is very difficult" -- was this supposed to be "reviewing without context" ?

pnkfelix (Dec 20 2019 at 14:35, on Zulip):

"However, we would likely “just approve” because it is fairly mechanical and doesn’t require a design meeting"

it would be good to clarify that this "just approve" is meant as a response to the associated major change issue , not to a PR opened with no associated major change issue. (Or at least, that is my inference of what the intent was in this example. If that is not the case, definitely needs clarification.)

nikomatsakis (Dec 20 2019 at 14:37, on Zulip):

was this supposed to be "reviewing without context" ?

yes, corrected

nikomatsakis (Dec 20 2019 at 14:39, on Zulip):

it would be good to clarify that this "just approve" is meant as a response to the associated major change issue , not to a PR opened with no associated major change issue. (Or at least, that is my inference of what the intent was in this example. If that is not the case, definitely needs clarification.)

tried to clarify the intent, thanks

pnkfelix (Dec 20 2019 at 14:40, on Zulip):

I've been musing about where the structural-match PR #67088 would go; i.e. if its a major change or not

oli (Dec 20 2019 at 14:40, on Zulip):

I have an example: "Heads up, @Santiago Pastorino is removing PlaceBase::Static breaking every single use site of place.base: https://github.com/rust-lang/rust/pull/67000"

pnkfelix (Dec 20 2019 at 14:41, on Zulip):

Re PR #67088, see in particular my comment here; I had written the PR under the assumption that the context had been well established in past issues. But maybe a fresh issue as a heads-up would have made things much smoother.

nikomatsakis (Dec 20 2019 at 14:43, on Zulip):

I have an example: "Heads up, Santiago Pastorino is removing PlaceBase::Static breaking every single use site of place.base: https://github.com/rust-lang/rust/pull/67000"

maybe add that to the hackmd :)

nikomatsakis (Dec 20 2019 at 14:44, on Zulip):

I would say that the answer that @mw and I were coming to was "if you are in doubt whether it's a major change, it probably is" -- but that in turn pushes us to wanting a very light-weight process

nikomatsakis (Dec 20 2019 at 14:44, on Zulip):

I think the premise was that, otherwise, this was only capturing a relatively narrow set of things and would be less useful

nikomatsakis (Dec 20 2019 at 15:01, on Zulip):

Hey @T-compiler/meeting -- design meeting starting now.

nikomatsakis (Dec 20 2019 at 15:01, on Zulip):

Announcements

nikomatsakis (Dec 20 2019 at 15:03, on Zulip):

I have a question -- I failed to produce minutes from the last two design meetings. I was planning on spending some time to go back and do that today. But I'm wondering if anybody else wants to take one of them =)

oli (Dec 20 2019 at 15:03, on Zulip):

on 27/28 December, @Wesley Wiser and I will be hacking on mir opt stuff. If anyone wants to join, they're more than welcome

nikomatsakis (Dec 20 2019 at 15:04, on Zulip):

ooh fun

nikomatsakis (Dec 20 2019 at 15:04, on Zulip):

what time?

nikomatsakis (Dec 20 2019 at 15:04, on Zulip):

I guess just "all day"? :)

oli (Dec 20 2019 at 15:04, on Zulip):

yea

nikomatsakis (Dec 20 2019 at 15:04, on Zulip):

I might join you in the mornings

nikomatsakis (Dec 20 2019 at 15:04, on Zulip):

well, my mornings

oli (Dec 20 2019 at 15:04, on Zulip):

hehe

pnkfelix (Dec 20 2019 at 15:05, on Zulip):

I have a question -- I failed to produce minutes from the last two design meetings. I was planning on spending some time to go back and do that today. But I'm wondering if anybody else wants to take one of them =)

I can work on making minutes

nikomatsakis (Dec 20 2019 at 15:05, on Zulip):

I've been working on writing up the design and justification of Chalk's abstract type library. There is a Zulip topic #wg-traits > visualizing the chalk-ir for discussing it (somewhat odd topic title, I suppose, it began with me posting a diagram I made). I'd love to start getting some feedback, though the write-up probably isn't quite far enough along to see the full picture.

nikomatsakis (Dec 20 2019 at 15:05, on Zulip):

(I should add some links to some of the design meeting hackmds where we've discussed some of the details, too.)

mw (Dec 20 2019 at 15:06, on Zulip):

@nikomatsakis is this interesting for people not working on Chalk/traits too?

mw (Dec 20 2019 at 15:06, on Zulip):

i.e. is it a "test case" for librarification?

nikomatsakis (Dec 20 2019 at 15:06, on Zulip):

Probably. I hope that -- when done -- this library would be part of the library-ification effort

nikomatsakis (Dec 20 2019 at 15:07, on Zulip):

The idea would be that it will be the basis for chalk and type checker libraries, and perhaps (eventually) referenced by MIR libraries too

nikomatsakis (Dec 20 2019 at 15:07, on Zulip):

(since MIR kind of needs a definition of types)

pnkfelix (Dec 20 2019 at 15:07, on Zulip):

I already hate LucidChart

nikomatsakis (Dec 20 2019 at 15:07, on Zulip):

lol

nikomatsakis (Dec 20 2019 at 15:07, on Zulip):

It's so far the best online charting tool I found? But that's not saying much

nikomatsakis (Dec 20 2019 at 15:08, on Zulip):

clearly I need to just hand-write SVG files

pnkfelix (Dec 20 2019 at 15:08, on Zulip):

(I'm just annoyed by how many barriers I've had to go through just to look at your picture)

nikomatsakis (Dec 20 2019 at 15:08, on Zulip):

ah

pnkfelix (Dec 20 2019 at 15:08, on Zulip):

I need to revive mon-artist

mw (Dec 20 2019 at 15:08, on Zulip):

clearly

nikomatsakis (Dec 20 2019 at 15:08, on Zulip):

yeah I had planned at some point to export an SVG and embed it in the gist :)

nikomatsakis (Dec 20 2019 at 15:09, on Zulip):

well, probably everybody is here...

nikomatsakis (Dec 20 2019 at 15:09, on Zulip):

any last announcements?

nikomatsakis (Dec 20 2019 at 15:09, on Zulip):

Based on the :wave: emojis above we have slightly less of a "quorum" than I would hope for

nikomatsakis (Dec 20 2019 at 15:09, on Zulip):

but that's ok

nikomatsakis (Dec 20 2019 at 15:10, on Zulip):

As noted in #t-compiler > pre-design meeting 2019-12-19, the plan is to discuss compiler-team#209, "major changes proposal". mw and I drafted up a hackmd here with some motivations and an initial draft proposal. The high-level summary, though, is as follows:

nikomatsakis (Dec 20 2019 at 15:10, on Zulip):

That's what I wrote above

nikomatsakis (Dec 20 2019 at 15:10, on Zulip):

I thnk it probably makes sense to spend some time walking through the hackmd

pnkfelix (Dec 20 2019 at 15:10, on Zulip):

how do you want this meeting to go: Did you want to review the text itself with everyone present, or can I start writing down concerns?

nikomatsakis (Dec 20 2019 at 15:11, on Zulip):

@pnkfelix maybe note them in the hackmd itself

nikomatsakis (Dec 20 2019 at 15:11, on Zulip):

(I'm guessing not everybody read it?)

nikomatsakis (Dec 20 2019 at 15:11, on Zulip):

So I guess the high-level sumamry is clear since I posted it like 22 times. I'll just start throwing out a bit the motivations -- I'd definitely like to hear if people think they are off base :)

nikomatsakis (Dec 20 2019 at 15:11, on Zulip):

Proposal is to add some sort of notification / lightweight process before making major changes.

nikomatsakis (Dec 20 2019 at 15:12, on Zulip):

In contrast, today there is no stated process for a "major change" beyond just opening a PR.
Of course some changes get a lot of discussion before hand, but others do not.

nikomatsakis (Dec 20 2019 at 15:12, on Zulip):

Some problems that we see with today's system:

nikomatsakis (Dec 20 2019 at 15:12, on Zulip):
nikomatsakis (Dec 20 2019 at 15:13, on Zulip):
nikomatsakis (Dec 20 2019 at 15:13, on Zulip):
nikomatsakis (Dec 20 2019 at 15:13, on Zulip):

I'll stop here for a second

nikomatsakis (Dec 20 2019 at 15:14, on Zulip):

Mostly I'm curious if these "problems" speak to other folks or not

pnkfelix (Dec 20 2019 at 15:15, on Zulip):

these are all indeed problems

pnkfelix (Dec 20 2019 at 15:15, on Zulip):

especially since some PR's subsequently get "auto-closed"

pnkfelix (Dec 20 2019 at 15:15, on Zulip):

due to lack of activity

nikomatsakis (Dec 20 2019 at 15:15, on Zulip):

Yeah.

nikomatsakis (Dec 20 2019 at 15:16, on Zulip):

One thing I would note is that I think some of the "delay" and limbo is inevitable --

pnkfelix (Dec 20 2019 at 15:16, on Zulip):

which, while good for health of project overall, is surely a bummer for the contrbitor

nikomatsakis (Dec 20 2019 at 15:16, on Zulip):

but I still think it's beneficial to represent that uncertainty in some way other than an open PR

simulacrum (Dec 20 2019 at 15:16, on Zulip):

fwiw, we do have pretty high control over that -- if we want to keep something open because compiler team is interested, we of course can

nikomatsakis (Dec 20 2019 at 15:16, on Zulip):

(but we can discuss in more detail that later I guess)

nikomatsakis (Dec 20 2019 at 15:16, on Zulip):

fwiw, we do have pretty high control over that -- if we want to keep something open because compiler team is interested, we of course can

I see it as a symptom of a problem and not the "root problem"

mw (Dec 20 2019 at 15:16, on Zulip):

yes it's a symptom that makes it pretty obvious that there's a problem

nikomatsakis (Dec 20 2019 at 15:17, on Zulip):

OK, the doc basically then points out that there are strengths to today's "system" (or lack thereof)

nikomatsakis (Dec 20 2019 at 15:17, on Zulip):

Some strengths of today's system that we would like to preserve:

nikomatsakis (Dec 20 2019 at 15:17, on Zulip):

in short, it's often super awesome that people just go out, hack up some PR, and come in with a fix to some problem

nikomatsakis (Dec 20 2019 at 15:17, on Zulip):

that seemed like it was never going to get fixed

nikomatsakis (Dec 20 2019 at 15:18, on Zulip):

and open source doesn't really lend itself to too much "top down" management somehow, or at least if you get too constraining, you're losing a lot of the potential benefits

nikomatsakis (Dec 20 2019 at 15:18, on Zulip):

The last part was our attempt to elaborate how we would know the system is working. It's working if:

simulacrum (Dec 20 2019 at 15:19, on Zulip):

I would also like to preserve some sort of "high throughput" if at all possible, e.g., we should not block on time at a meeting or so I think (or at least try to avoid that mode)

nikomatsakis (Dec 20 2019 at 15:19, on Zulip):

good point, we should add that to the list

nikomatsakis (Dec 20 2019 at 15:19, on Zulip):

i.e., it's working if we haven't just added a lot of overhead and blocking

pnkfelix (Dec 20 2019 at 15:19, on Zulip):

the "major change issue" will also be a good place to track the progress if a change ends up being implemented as a series of PR's (either due to drafts being closed and fresh ones opened, or if there is a chain of PR's that depend on each other)

mw (Dec 20 2019 at 15:20, on Zulip):

I would also like to preserve some sort of "high throughput" if at all possible, e.g., we should not block on time at a meeting or so I think (or at least try to avoid that mode)

yes, it would be good if a "this is harmless, go ahead" decision could always be made quickly

pnkfelix (Dec 20 2019 at 15:20, on Zulip):

we should not block on time at a meeting

what does this mean, precisely?

pnkfelix (Dec 20 2019 at 15:20, on Zulip):

as in, we should not cause major change issues to lie stagnant because we ran out of time as synchronous meeting point?

simulacrum (Dec 20 2019 at 15:21, on Zulip):

I think it may mean that we do not block on a meeting at all

pnkfelix (Dec 20 2019 at 15:21, on Zulip):

I still don't understand what that means

nikomatsakis (Dec 20 2019 at 15:21, on Zulip):

OK, that definitely runs afoul of the process I proposed :)

simulacrum (Dec 20 2019 at 15:21, on Zulip):

basically, if you have a reviewer, and you've documented things, then we should be able to async move forward

nikomatsakis (Dec 20 2019 at 15:21, on Zulip):

(but I think that's precisely what we should be hammering out)

simulacrum (Dec 20 2019 at 15:21, on Zulip):

unless someone raises a concern

simulacrum (Dec 20 2019 at 15:22, on Zulip):

the default I think should be "major changes need a partner" essentially

nikomatsakis (Dec 20 2019 at 15:22, on Zulip):

let's note that down

nikomatsakis (Dec 20 2019 at 15:22, on Zulip):

maybe I'll lay out the "strawperson" change, I think we could incorporate something like that that I would e comfortable with

mw (Dec 20 2019 at 15:22, on Zulip):

that sounds like a good idea

pnkfelix (Dec 20 2019 at 15:22, on Zulip):

@simulacrum so to be clear: the developer would still be expected to file a major change issue

simulacrum (Dec 20 2019 at 15:22, on Zulip):

yes

pnkfelix (Dec 20 2019 at 15:22, on Zulip):

and it would be listed in whatever regular update process we have

simulacrum (Dec 20 2019 at 15:22, on Zulip):

correct, yes, but we would not expect the developer to block on approval

simulacrum (Dec 20 2019 at 15:23, on Zulip):

(unless they don't have a partner)

simulacrum (Dec 20 2019 at 15:23, on Zulip):

basically, this is similar to the shepherd finding their lang team partner in my mind (though a bit different)

mw (Dec 20 2019 at 15:23, on Zulip):

well, I think we are missing a step here. when is something classified as a major change?

nikomatsakis (Dec 20 2019 at 15:24, on Zulip):

yeah, let's file this (I took some notes at end of hackmd) and bring it back up later ?

mw (Dec 20 2019 at 15:24, on Zulip):

but let's maybe let @nikomatsakis drive the discussion

simulacrum (Dec 20 2019 at 15:24, on Zulip):

I would personally not want to spend time figuring out what classifies as a major change, and plat that by ear -- I don't think it's a super important question, we can figure out what's best as we go

nikomatsakis (Dec 20 2019 at 15:24, on Zulip):

well I see two options:

nikomatsakis (Dec 20 2019 at 15:24, on Zulip):
nikomatsakis (Dec 20 2019 at 15:24, on Zulip):

I'll just briefly cover the process from doc that was proposed:

nikomatsakis (Dec 20 2019 at 15:25, on Zulip):

I think that's the heart of it, right

pnkfelix (Dec 20 2019 at 15:25, on Zulip):

To be clear, a reviewer is not expected to be a pair-programmer

pnkfelix (Dec 20 2019 at 15:25, on Zulip):

right?

nikomatsakis (Dec 20 2019 at 15:25, on Zulip):

I guess the point is that it may not be approved, there may be pushback, or request for a design meeting, better docs, etc

nikomatsakis (Dec 20 2019 at 15:25, on Zulip):

that's correct

nikomatsakis (Dec 20 2019 at 15:26, on Zulip):

I would personally not want to spend time figuring out what classifies as a major change, and plat that by ear -- I don't think it's a super important question, we can figure out what's best as we go

I think we should spend at least some time on this

nikomatsakis (Dec 20 2019 at 15:26, on Zulip):

in particular I think there could be a lot of different expectations

nikomatsakis (Dec 20 2019 at 15:26, on Zulip):

but @mw and I were thinking of a fairly expansive definition

nikomatsakis (Dec 20 2019 at 15:27, on Zulip):

though I think an important point is that -- for larger efforts like e.g. the MIR place refactorings -- there would probably be many PRs, but one "major change issue"

simulacrum (Dec 20 2019 at 15:27, on Zulip):

Maybe something like "a change that (should) modify rustc-guide documentation"? To me that feels about the right level

nikomatsakis (Dec 20 2019 at 15:27, on Zulip):

Interesting

nikomatsakis (Dec 20 2019 at 15:28, on Zulip):

I rather like that, though it presumes I think a better level of rustc-guide docs than we have today :)

pnkfelix (Dec 20 2019 at 15:28, on Zulip):

(even if it doesn't, if you feel like "aww it should have ...")

nikomatsakis (Dec 20 2019 at 15:28, on Zulip):

I do agree that we can develop this over time -- I'd sort of rather that, initially, we e.g. bring it up at triage meetings if in doubt

nikomatsakis (Dec 20 2019 at 15:28, on Zulip):

or something like that

nikomatsakis (Dec 20 2019 at 15:28, on Zulip):

(or maybe just in a zulip topic, doesn't have to be meetings)

nikomatsakis (Dec 20 2019 at 15:29, on Zulip):

there was another point that @mw and I said informally

nikomatsakis (Dec 20 2019 at 15:29, on Zulip):

that I don't think made it into the doc

nikomatsakis (Dec 20 2019 at 15:29, on Zulip):

but which I want to bring up

nikomatsakis (Dec 20 2019 at 15:29, on Zulip):

which was -- what happens if someboyd opens a PR but they haven't done this process

nikomatsakis (Dec 20 2019 at 15:29, on Zulip):

one option might be that we have a little nice bit of text about the process

pnkfelix (Dec 20 2019 at 15:29, on Zulip):

(and what do we do with the current set of open PR's, too.)

nikomatsakis (Dec 20 2019 at 15:29, on Zulip):

and we said "this seemsl ike it is a major change; I'm going to close for now, can you file an issue?"

nikomatsakis (Dec 20 2019 at 15:30, on Zulip):

one can also imagine that closing is "too rude" though I don't think it necessarily has to be. It depends on how much we want the open PRs to be kept "tidy"

pnkfelix (Dec 20 2019 at 15:30, on Zulip):

also, should high priority items be given more flexibility?

pnkfelix (Dec 20 2019 at 15:30, on Zulip):

e.g. if something is being targetted as a fix for beta

mw (Dec 20 2019 at 15:30, on Zulip):

I think if it's part of a well defined process it's not so rude

pnkfelix (Dec 20 2019 at 15:31, on Zulip):

(even if its a major change; which is an exceptional case, I'll admit...)

simulacrum (Dec 20 2019 at 15:31, on Zulip):

I think one approach is also -- if we go with my proposal for how to categorize these things -- we ask them to write up the rustc-guide docs that would go along with their change

simulacrum (Dec 20 2019 at 15:31, on Zulip):

this sort of helps guide "what is the content of the issue"

nikomatsakis (Dec 20 2019 at 15:31, on Zulip):

I think @simulacrum that could be a good part of the text, yeah

simulacrum (Dec 20 2019 at 15:31, on Zulip):

and keeps them interested, hopefully, as its a meaningful step (and likely a prereq for our discussion)

nikomatsakis (Dec 20 2019 at 15:31, on Zulip):

i.e., we point them at the process, and it points out that we probably need docs

nikomatsakis (Dec 20 2019 at 15:31, on Zulip):

however, a lot of times the bootstrapping there is hard

nikomatsakis (Dec 20 2019 at 15:31, on Zulip):

but I feel ok about that

nikomatsakis (Dec 20 2019 at 15:32, on Zulip):

that is, as an example, I would feel some obligation to help write the "base material" in many cases

Wesley Wiser (Dec 20 2019 at 15:32, on Zulip):

I think there could be a friendly comment which just says they need to write up a proposal before their change can be merged and then if they don't do that within a week (or two?), we could close the PR per the policy.

nikomatsakis (Dec 20 2019 at 15:32, on Zulip):

and I wouldn't view it as a prereq to starting work, more something to be done as we go

nikomatsakis (Dec 20 2019 at 15:32, on Zulip):

(and probably together)

pnkfelix (Dec 20 2019 at 15:32, on Zulip):

@Wesley Wiser 's idea is interesting

simulacrum (Dec 20 2019 at 15:32, on Zulip):

I see it as a pre-req potentially to us discussing/understanding the change

pnkfelix (Dec 20 2019 at 15:32, on Zulip):

delaying closing the PR, giving people the chance to sort of "backfill" the major change proposal.

nikomatsakis (Dec 20 2019 at 15:33, on Zulip):

I see it as a pre-req potentially to us discussing/understanding the change

what is "it" here? writing up rustc-guide docs?

simulacrum (Dec 20 2019 at 15:33, on Zulip):

fwiw, the two weeks is I think what we normally give any PR that's blocked on the author

simulacrum (Dec 20 2019 at 15:33, on Zulip):

yes, rustc-guide docs

mw (Dec 20 2019 at 15:33, on Zulip):

and I wouldn't view it as a prereq to starting work, more something to be done as we go

yes, the process would not stop anyone from experimenting. it's more about setting expectations about what needs to happen before something can get merged

nikomatsakis (Dec 20 2019 at 15:33, on Zulip):

I guess I think that it's often possible to convey the "gist" of an idea more concisely, though of course it'd be beter if rustc-guide contained even sparse sentences than nothing

nikomatsakis (Dec 20 2019 at 15:33, on Zulip):

but ok let's focus on this question:

nikomatsakis (Dec 20 2019 at 15:33, on Zulip):

do

nikomatsakis (Dec 20 2019 at 15:34, on Zulip):

/poll When a PR is opened that is not following policy, do we?

pnkfelix (Dec 20 2019 at 15:34, on Zulip):

don't those polls not work on mobile?

nikomatsakis (Dec 20 2019 at 15:34, on Zulip):

Oh, I don't know

nikomatsakis (Dec 20 2019 at 15:34, on Zulip):

I was hoping that had bene fixed :)

pnkfelix (Dec 20 2019 at 15:34, on Zulip):

(that was why I use emojis in triage meeting)

simulacrum (Dec 20 2019 at 15:34, on Zulip):

one question -- the nice message would be "immediate", right?

davidtwco (Dec 20 2019 at 15:35, on Zulip):

don't those polls not work on mobile?

I think they just say "there's a poll here, go to a desktop to see it".

nikomatsakis (Dec 20 2019 at 15:35, on Zulip):

yes, I meant that the nice message would be immediate

nikomatsakis (Dec 20 2019 at 15:35, on Zulip):

ok so here is one other edge case

nikomatsakis (Dec 20 2019 at 15:35, on Zulip):

suppose that

nikomatsakis (Dec 20 2019 at 15:35, on Zulip):

I geuss we can discuss case by case maybe

pnkfelix (Dec 20 2019 at 15:36, on Zulip):

(if you are here and are on mobile and cannot see the poll, add to a :mobile_phone: :mobile_phone: emoji on this message)

nikomatsakis (Dec 20 2019 at 15:36, on Zulip):

I'm sort of inclined to say though that if we decide "this needs a design meeting" or something we can close the PR

nikomatsakis (Dec 20 2019 at 15:36, on Zulip):

but I don't have a strong opinion really, maybe ther'es not much point

simulacrum (Dec 20 2019 at 15:36, on Zulip):

I would agree with that -- and would prefer that we do in fact close the PR

simulacrum (Dec 20 2019 at 15:36, on Zulip):

an open PR creates a mild expectation that others should not touch the area of code, etc.

simulacrum (Dec 20 2019 at 15:36, on Zulip):

(due to rebase pain)

nikomatsakis (Dec 20 2019 at 15:37, on Zulip):

yes

nikomatsakis (Dec 20 2019 at 15:37, on Zulip):

ok, great

pnkfelix (Dec 20 2019 at 15:37, on Zulip):

yes I think it makes sense to let the majorness of the change to guide whether to close immediately or not

nikomatsakis (Dec 20 2019 at 15:37, on Zulip):
nikomatsakis (Dec 20 2019 at 15:37, on Zulip):

I added that to the document

nikomatsakis (Dec 20 2019 at 15:37, on Zulip):

and I will take this opportunity to complain about Zulip and nested bullet lists

pnkfelix (Dec 20 2019 at 15:37, on Zulip):

(and if we cannot immediately tell what the majorness is, then we should bias towards closing, of course?)

nikomatsakis (Dec 20 2019 at 15:37, on Zulip):

as I always do

nikomatsakis (Dec 20 2019 at 15:38, on Zulip):

(and if we cannot immediately tell what the majorness is, then we should bias towards closing, of course?)

yeah, I phrased it as "it is not green lighted", which I think implies such a bias

nikomatsakis (Dec 20 2019 at 15:38, on Zulip):

OK, having settled that, I'm trying to decide what to turn to next :)

Wesley Wiser (Dec 20 2019 at 15:38, on Zulip):

I think it would be good to have a minimum number of "approvals" required (2 perhaps?).

Without that, I could see team members being less willing to provide an approval unless they were 100% sure about all of the details off the top of their head. Requiring only one approval puts a lot of (perceived) responsibility on that one person.

For contributors, making a large change could be a very expensive time commitment and I think they would feel better about the odds of their PR getting approved if multiple people have signed off on the idea.

nikomatsakis (Dec 20 2019 at 15:39, on Zulip):

Hmm interesting

nikomatsakis (Dec 20 2019 at 15:39, on Zulip):

I feel like

nikomatsakis (Dec 20 2019 at 15:39, on Zulip):

I guess I feel like there are some changes that I would feel comfortable approving all by myself

simulacrum (Dec 20 2019 at 15:39, on Zulip):

I would feel like we should not increase the number of required approvals, but note that of course the more the better :)

nikomatsakis (Dec 20 2019 at 15:39, on Zulip):

but maybe I'm exceptional :P

simulacrum (Dec 20 2019 at 15:39, on Zulip):

i.e., if you may not want to do it if it will plausibly get declined, then you should perhaps wait for approvals (and indicate as such)

nikomatsakis (Dec 20 2019 at 15:39, on Zulip):

I guess I tend to think we should not require 2 approvals, but there should be a way for someone to say "I approve, and will review, but I'd like someone else from compiler-team to give their assent since I'm not 100% sure"

pnkfelix (Dec 20 2019 at 15:39, on Zulip):

what set of people are part of the reviewers here?

pnkfelix (Dec 20 2019 at 15:40, on Zulip):

all contributors? just compiler-team members?

nikomatsakis (Dec 20 2019 at 15:40, on Zulip):

good question

nikomatsakis (Dec 20 2019 at 15:40, on Zulip):

maybe contributors can review, but at least 1 member must asseent

pnkfelix (Dec 20 2019 at 15:40, on Zulip):

its possible that the number of approvals required could depend on "seniority"

nikomatsakis (Dec 20 2019 at 15:40, on Zulip):

or something like that

nikomatsakis (Dec 20 2019 at 15:40, on Zulip):

seems right to me

mw (Dec 20 2019 at 15:40, on Zulip):

"require" might be too strong, but it should be encouraged to ask for additional eyes, if in doubt

Wesley Wiser (Dec 20 2019 at 15:40, on Zulip):

I think @nikomatsakis's comment gets at what I was trying to describe for team members better than I did.

nikomatsakis (Dec 20 2019 at 15:41, on Zulip):

I like the idea of having a few possible responses

nikomatsakis (Dec 20 2019 at 15:42, on Zulip):

side note

mw (Dec 20 2019 at 15:42, on Zulip):

"approval for further experimentation" might also be good

nikomatsakis (Dec 20 2019 at 15:42, on Zulip):

it is possible to create Zulip "bridges"

nikomatsakis (Dec 20 2019 at 15:43, on Zulip):

one way is via e-mail, but there are probably things for "opened GH issues" etc

pnkfelix (Dec 20 2019 at 15:43, on Zulip):

it is possible to create Zulip "bridges"

was this for a different topic ?

nikomatsakis (Dec 20 2019 at 15:43, on Zulip):

I thikn we should have a dedicated Zulip streawm

nikomatsakis (Dec 20 2019 at 15:43, on Zulip):

where each such issue automatically creates a topic in the stream

nikomatsakis (Dec 20 2019 at 15:43, on Zulip):

(if we can arrange it)

pnkfelix (Dec 20 2019 at 15:43, on Zulip):

ah okay I see now

nikomatsakis (Dec 20 2019 at 15:43, on Zulip):

sorry, I'm just kind if brainstorming about how to encourage conversation and prompted feedback :)

pnkfelix (Dec 20 2019 at 15:43, on Zulip):

/me really wants to start learning more about the Zulip API

nikomatsakis (Dec 20 2019 at 15:44, on Zulip):

anyway, as to what @mw said, I guess it would be something like "I approve but only on an experimental basis" ?

nikomatsakis (Dec 20 2019 at 15:44, on Zulip):

as a further option

nikomatsakis (Dec 20 2019 at 15:44, on Zulip):

that's kind of an orthogonal thing

nikomatsakis (Dec 20 2019 at 15:44, on Zulip):
nikomatsakis (Dec 20 2019 at 15:44, on Zulip):

i.e., you could be a reviewier but still think it shoul be experimental :)

pnkfelix (Dec 20 2019 at 15:45, on Zulip):

where "experimental" means: "it can land but only guarded under a -Z flag" ?

mw (Dec 20 2019 at 15:45, on Zulip):

yeah, it's not entirely orthogonal I think

mw (Dec 20 2019 at 15:45, on Zulip):

where "experimental" means: "it can land but only guarded under a -Z flag" ?

for example

mw (Dec 20 2019 at 15:46, on Zulip):

or if there is an unmerged proof of concept, but more fleshed out

simulacrum (Dec 20 2019 at 15:46, on Zulip):

I approve the overall idea but not necessarily the specific design?

simulacrum (Dec 20 2019 at 15:46, on Zulip):

(as what "experimental" means)

pnkfelix (Dec 20 2019 at 15:47, on Zulip):

does that then imply we need a design meeting?

mw (Dec 20 2019 at 15:47, on Zulip):

I was thinking of cases where there might be edge cases with bad performance and potential solutions that need to be tested first

pnkfelix (Dec 20 2019 at 15:47, on Zulip):

or do you more mean "I have specific points of feedback to fix the design; but it doesn't need larger group discussion" ?

simulacrum (Dec 20 2019 at 15:47, on Zulip):

well, more so, that you're not sure if the design is good, but want to see an implementation to decide

mw (Dec 20 2019 at 15:48, on Zulip):

yes, that's another good case

nikomatsakis (Dec 20 2019 at 15:50, on Zulip):

(sorry, had to drop afk unexpectedly, catching up)

nikomatsakis (Dec 20 2019 at 15:52, on Zulip):

well it seems like we need some form of experimentation

nikomatsakis (Dec 20 2019 at 15:52, on Zulip):

which could mean a lot of things

nikomatsakis (Dec 20 2019 at 15:52, on Zulip):

it might be just like a "caveats" section

nikomatsakis (Dec 20 2019 at 15:53, on Zulip):

i.e., the options I originally listed, plus some way to

nikomatsakis (Dec 20 2019 at 15:53, on Zulip):

add notes

nikomatsakis (Dec 20 2019 at 15:54, on Zulip):

ok well it's 54 minutes in

nikomatsakis (Dec 20 2019 at 15:55, on Zulip):

I don't see anybody still commenting :) not sure though if that's my internet or what

simulacrum (Dec 20 2019 at 15:55, on Zulip):

no, no comments :)

nikomatsakis (Dec 20 2019 at 15:55, on Zulip):

ok :)

nikomatsakis (Dec 20 2019 at 15:55, on Zulip):

my computer was being weird, but good

mw (Dec 20 2019 at 15:55, on Zulip):

everything's settled

nikomatsakis (Dec 20 2019 at 15:55, on Zulip):

well seems like we're...done-ish?

pnkfelix (Dec 20 2019 at 15:56, on Zulip):

there was something I had thought we should announce, but I've since forgotten what it was ...

pnkfelix (Dec 20 2019 at 15:56, on Zulip):

oh!

pnkfelix (Dec 20 2019 at 15:56, on Zulip):

next weeks' meetings

pnkfelix (Dec 20 2019 at 15:56, on Zulip):

we should cancel them, right?

nikomatsakis (Dec 20 2019 at 15:57, on Zulip):

Yes :)

nikomatsakis (Dec 20 2019 at 15:57, on Zulip):

Good point

nikomatsakis (Dec 20 2019 at 15:57, on Zulip):

The next planning meeting we scheduled for 2 weeks from now already

mw (Dec 20 2019 at 15:57, on Zulip):

we should cancel them, right?

which makes this the last meeting in this decade

pnkfelix (Dec 20 2019 at 15:57, on Zulip):

Alright, Rust is all finished.

nikomatsakis (Dec 20 2019 at 15:58, on Zulip):

@simulacrum to follow up briefly on rustc-guide -- I agree we should encourage folks to write-up rustc-guide docs -- but i think sometimes that could be a hard bar, esp. if there's not much to start with, but even just having '"bullet point docs" can be a great starting point. Mostly I thkn we sohuld encouage them to open an issue first and foremost, and (asynchronously) we should work on getting docs done

nikomatsakis (Dec 20 2019 at 15:58, on Zulip):

but that should definitely be part of landing the change overall

simulacrum (Dec 20 2019 at 15:58, on Zulip):

I would sort of agree -- I guess a lot of the time it is a high bar, but I sometimes feel like it is sort of necessary

nikomatsakis (Dec 20 2019 at 15:58, on Zulip):

(I guess it all depends on the change)

pnkfelix (Dec 20 2019 at 15:59, on Zulip):

I definitely have had cases where someone said "this needs documentation change"

mw (Dec 20 2019 at 15:59, on Zulip):

good module level docs might be appropriate and sufficient too

pnkfelix (Dec 20 2019 at 15:59, on Zulip):

and then I go to look, and the underlying feature is undocumented

nikomatsakis (Dec 20 2019 at 15:59, on Zulip):

I would sort of agree -- I guess a lot of the time it is a high bar, but I sometimes feel like it is sort of necessary

Yes. I guess I agree with you too :) I think we should be starting to raise the bar, end of day

nikomatsakis (Dec 20 2019 at 15:59, on Zulip):

it's just more about the "ordering"

nikomatsakis (Dec 20 2019 at 15:59, on Zulip):

but I shouldn't push back too hard, after all, I want more docs :)

simulacrum (Dec 20 2019 at 15:59, on Zulip):

@pnkfelix strong agree there :) I think if we don't do more to require docs upfront we'll perpetually not really have docs for many things

nikomatsakis (Dec 20 2019 at 16:00, on Zulip):

Yeah, this seems right.

nikomatsakis (Dec 20 2019 at 16:01, on Zulip):

all right, thanks all! :heart:

Santiago Pastorino (Dec 20 2019 at 16:01, on Zulip):

simulacrum to follow up briefly on rustc-guide -- I agree we should encourage folks to write-up rustc-guide docs -- but i think sometimes that could be a hard bar, esp. if there's not much to start with, but even just having '"bullet point docs" can be a great starting point. Mostly I thkn we sohuld encouage them to open an issue first and foremost, and (asynchronously) we should work on getting docs done

in some way the Learning WG could help I guess

mw (Dec 20 2019 at 16:02, on Zulip):

Thanks for driving the meeting, @nikomatsakis !

simulacrum (Dec 20 2019 at 16:02, on Zulip):

Sure -- but I don't think putting it on that WG to write up docs is the right approach for the most part

Santiago Pastorino (Dec 20 2019 at 16:02, on Zulip):

yeah no no, I was clarifying a bit more

simulacrum (Dec 20 2019 at 16:02, on Zulip):

I think it's excellent that they exist, but should hopefully be more of a review task force and maybe bootstrapping helpers rather than "I write code, they document" :)

simulacrum (Dec 20 2019 at 16:03, on Zulip):

but sure I agree

Santiago Pastorino (Dec 20 2019 at 16:03, on Zulip):

yeah no

nikomatsakis (Dec 20 2019 at 16:03, on Zulip):

I think @simulacrum mostly what I'm saying is two things:

nikomatsakis (Dec 20 2019 at 16:03, on Zulip):
Santiago Pastorino (Dec 20 2019 at 16:03, on Zulip):

maybe having the person writing some bullet points and the WG in some way helping would be great

simulacrum (Dec 20 2019 at 16:03, on Zulip):

I would agree with that. I guess I see it as "explain your change" -- though what that looks like varies on the change

nikomatsakis (Dec 20 2019 at 16:03, on Zulip):

I guess I think the role of the WG should be more in paying down our "doc debt"

simulacrum (Dec 20 2019 at 16:04, on Zulip):

e.g. with some of the parallel patches I wrote docs after the PR was reviewed

nikomatsakis (Dec 20 2019 at 16:04, on Zulip):

though longer term I do sort of love the idea of folks who

simulacrum (Dec 20 2019 at 16:04, on Zulip):

which I think worked well

nikomatsakis (Dec 20 2019 at 16:04, on Zulip):

contribute to rustc (and become experts on it) by helping to document it

nikomatsakis (Dec 20 2019 at 16:04, on Zulip):

which I think worked well

yes, agreed

nikomatsakis (Dec 20 2019 at 16:04, on Zulip):

if the change is small enough, a few bullets and high-level description before + more detailed docs after is great

nikomatsakis (Dec 20 2019 at 16:05, on Zulip):

ok, I have to go run to the bakery and get some bread, thanks again

Santiago Pastorino (Dec 20 2019 at 16:05, on Zulip):

"I write code, they document" -> they way I'd like to see this is a contributor helps a member of the learning wg to understand some area of the compiler and the member of the wg helps to get some documentation in

Santiago Pastorino (Dec 20 2019 at 16:05, on Zulip):

win - win

simulacrum (Dec 20 2019 at 16:05, on Zulip):

sure -- that works -- I just meant we can't expect to indefinitely offload that work :)

Santiago Pastorino (Dec 20 2019 at 16:05, on Zulip):

definitely :)

nikomatsakis (Dec 20 2019 at 17:52, on Zulip):

BTW, I updated the hackmd with what I believe to be the final consensus we arrived at

nikomatsakis (Dec 20 2019 at 17:53, on Zulip):

I'm curious what's the next step somehow :)

nikomatsakis (Dec 20 2019 at 17:53, on Zulip):

I coudl see writing up the docs on this in an end-user way as a PR to the forge

nikomatsakis (Dec 20 2019 at 17:53, on Zulip):

and then doing a FCP

simulacrum (Dec 20 2019 at 17:55, on Zulip):

I think maybe it makes sense to RFC it? I could imagine basically taking those bullet points and splitting them up into the appropriate sections

simulacrum (Dec 20 2019 at 17:55, on Zulip):

that seems like the most user-visible way we have

nikomatsakis (Dec 20 2019 at 17:55, on Zulip):

Yeah, that's reasonable

nikomatsakis (Dec 20 2019 at 17:56, on Zulip):

I like that

centril (Dec 20 2019 at 18:50, on Zulip):

Missed the meeting, but I have some thoughts:

nikomatsakis (Dec 20 2019 at 18:54, on Zulip):

Good thoughts. Some responses:

nikomatsakis (Dec 20 2019 at 18:55, on Zulip):

You're probably right that it's not necessary to get overly formal re: "contributor" vs "member". The most imporant point is how confident you are that (a) the idea is good and (b) others will agree =)

centril (Dec 20 2019 at 18:56, on Zulip):

The most imporant point is how confident you are that (a) the idea is good and (b) others will agree =)

Yeah basically :+1:

nikomatsakis (Dec 20 2019 at 18:57, on Zulip):

Regarding what constitutes a major change, I guess I think the example could go either way, depending on the details. There will always be some guesswork. I think we'll come to a better understanding over time. I would expect to start by erring on the side of "yes", though -- and perhaps just keep that as the rule. ("If in doubt...")

nikomatsakis (Dec 20 2019 at 18:57, on Zulip):

To address your specific example: I imagine that splitting a file and small-bore refactorings probably don't affect the documentation in rustc-guide

nikomatsakis (Dec 20 2019 at 18:57, on Zulip):

or if they do, it's with small tweaks like changing the filename, but not the real structure

nikomatsakis (Dec 20 2019 at 18:58, on Zulip):

the rustc-guide docs are meant to document the "high-level strategy", so for the parser, I would imagine they might talk about

nikomatsakis (Dec 20 2019 at 18:58, on Zulip):

that sort of thing

centril (Dec 20 2019 at 18:58, on Zulip):

(To take an example of where formality would be suboptimal, I think that e.g. arielb1 would be quite competent to r+ larger changes though they are neither formally contributor or team member.)

nikomatsakis (Dec 20 2019 at 18:58, on Zulip):

so if you refactored such a mechanism, then yes, I think it's a major change

nikomatsakis (Dec 20 2019 at 18:59, on Zulip):

well, I don't agree with that

nikomatsakis (Dec 20 2019 at 18:59, on Zulip):

I mean yes they are obviously competent

nikomatsakis (Dec 20 2019 at 18:59, on Zulip):

but note that they were also a compiler team member:)

nikomatsakis (Dec 20 2019 at 18:59, on Zulip):

so maybe they're a special case

centril (Dec 20 2019 at 18:59, on Zulip):

heh ^^

nikomatsakis (Dec 20 2019 at 18:59, on Zulip):

in general, though, if there is someone who is an expert and they're not a member

nikomatsakis (Dec 20 2019 at 18:59, on Zulip):

we should fix that by making them a member :)

nikomatsakis (Dec 20 2019 at 18:59, on Zulip):

but yeah I guess I can imagine

nikomatsakis (Dec 20 2019 at 18:59, on Zulip):

I mean I thnk it's fine to say "so and so reviews this PR because they wrote the code"

nikomatsakis (Dec 20 2019 at 19:00, on Zulip):

but they're not (in general) active enough to rise to contributor/member level

nikomatsakis (Dec 20 2019 at 19:00, on Zulip):

sure

nikomatsakis (Dec 20 2019 at 19:00, on Zulip):

I take back what I said :)

nikomatsakis (Dec 20 2019 at 19:00, on Zulip):

I guess the only thing is that I think we should try to be careful about who is r+'ing

nikomatsakis (Dec 20 2019 at 19:00, on Zulip):

but I agree that there are "edge cases" and the formal categories don't always fit

centril (Dec 20 2019 at 19:02, on Zulip):

OK so e.g. for the parser PR, changes to the rustc guide would be more about the fundamental mechanisms and stuff

centril (Dec 20 2019 at 19:03, on Zulip):

Though I think that cleaning up a part of the code-base is a really great way to gain understanding, improve the local documentation, and then it can eventually go into the rustc guide

nikomatsakis (Dec 20 2019 at 19:03, on Zulip):

I thnk so yes

nikomatsakis (Dec 20 2019 at 19:03, on Zulip):

I would also say

nikomatsakis (Dec 20 2019 at 19:03, on Zulip):

well first off I was going to add this note to the minutes:

nikomatsakis (Dec 20 2019 at 19:04, on Zulip):
nikomatsakis (Dec 20 2019 at 19:05, on Zulip):

I guess the point is that it's pretty easy to move forward

nikomatsakis (Dec 20 2019 at 19:05, on Zulip):

if you feel it's clearly good + others will agree

nikomatsakis (Dec 20 2019 at 19:06, on Zulip):

Though I think that cleaning up a part of the code-base is a really great way to gain understanding, improve the local documentation, and then it can eventually go into the rustc guide

yes for sure

nikomatsakis (Dec 20 2019 at 19:06, on Zulip):

if such a PR seems to cross the line, then it seems ok to quickly create an issue with some notes on the cleanup and just add a Closes rust-lang/compiler-team#123 and r+ the PR

nikomatsakis (Dec 20 2019 at 19:07, on Zulip):

(I am imagining then that this will still trigger a post or something so that people who care can have a chance to look in)

centril (Dec 20 2019 at 19:08, on Zulip):

Yea, makes sense :+1:

pnkfelix (Dec 20 2019 at 19:21, on Zulip):

wrt. code quality, giant 300 LOC methods are ungreat.

(bah, 300 LoC is nothing! now, 3 KLoC, then we're talking!)

simulacrum (Dec 20 2019 at 19:44, on Zulip):

@centril to add to what I said a bit -- when I said "Affect rustc-guide docs" I meant mostly not that it would just change them, but, if we had such docs today, then it would make them stale basically

simulacrum (Dec 20 2019 at 19:45, on Zulip):

so e.g. refactoring the parser probably wouldn't do this

simulacrum (Dec 20 2019 at 19:45, on Zulip):

because you're not changing the high-level design

simulacrum (Dec 20 2019 at 19:45, on Zulip):

but if you are then I think an issue and related overhead is reasonable

simulacrum (Dec 20 2019 at 19:45, on Zulip):

but because of the 'easy' policy, you'd not need to do much more than just an issue, because presumably e.g. estebank or petrochenkov would fairly quickly sign-off

simulacrum (Dec 20 2019 at 19:46, on Zulip):

(and if not, well, then maybe it's a good thing that we paused :)

centril (Dec 20 2019 at 21:12, on Zulip):

(bah, 300 LoC is nothing! now, 3 KLoC, then we're talking!)

oh lord please have mercy ;)

mark-i-m (Dec 21 2019 at 00:36, on Zulip):

I think maybe it makes sense to RFC it? I could imagine basically taking those bullet points and splitting them up into the appropriate sections

FWIW, I think we would need to update the rustc-guide with this procedure (particularly, part 1)

nikomatsakis (Dec 23 2019 at 11:43, on Zulip):

the major change policy is itself a major change, you're saying? ;)

mark-i-m (Dec 28 2019 at 22:58, on Zulip):

Yes lol
We have a couple of chapters on procedural stuff like what an average contribution looks like etc. It would be good to have a short chapter or at least a note that if you are planning a large change, you should go through such a process so as not to waste your time or the reviewer's time

Last update: Jan 21 2020 at 09:10UTC