On improving communication between proposal authors and reviewers

Hi everybody. I'm quite nervous about writing this post, but there was an issue when discussing one of the latest proposals, where some reviewers couldn’t agree with the authors on how much background information should be explained in the proposal and how educational in general the proposal should be. I’m hesitant to link to the discussion, but I feel that the problem raised there did not come to a conclusive understanding of what reviewers can possibly expect from proposal authors and vice versa, and it seemed to me that some feelings were unnecessarily hurt because of this.

I personally understand both parties though, and I feel that the whole community would benefit if we could provide a way to enhance the communication. I’m not sure if the discussion should be repeated though, so I quickly summarise the points being argued by the parties:

  • it is generally expected (or feels to me that way) that a reviewer has to be very comfortable with the topic being discussed, possibly having encountered its counterpart in other programming languages or libraries;
  • reviewers, however, might lack mechanisms to self-audit their familiarity with the topic;
  • proposal authors generally neither can preemptively address all possible questions in the proposal, nor dilute the discussion by answering them during the review as this can be counterproductive;
  • depending on the proposal topic, however, it might turn out that the possession of compiler and/or language development skills is quite crucial for a successful discussion, while the broader community is generally comprised of members who are only familiar with the language as a user.

There might be many ways to address this, however, I feel that I have a relatively low-effort solution that might still be quite effective: introduce a "Prior knowledge" section to the proposal template.

This section would contain a short list of concepts, links to educational resources and possibly past proposals/discussions that the proposal authors deem to be effective in introducing a reviewer to the topic. This helps both parties and achieves several goals:

  • relieves the need of directly writing explanations from proposal authors,
  • ensures that the proposal authors are on the same page with reviewers,
  • ensures that reviewers get acquainted with the topic directly from the best resources out there, as otherwise quite often people need to rely on serendipity rather than their sheer passion to learn,
  • provides a way for reviewers to privately audit their knowledge instead of being struck by a comment on their ignorance,
  • if this section is filled before the proposal is scheduled for review, reviewers might have enough time to properly introduce themselves to the topic to be more effective in reviewing and only ask additional questions if the provided material hasn’t answered them,
  • allows for other suggestions to the list via a simple PR,
  • provides educational value for the community even past review, while still staying on topic,
  • reserves the “Alternatives considered” section for more topical handling of the said alternatives.

Please let me know what you think. This still requires some additional effort from proposal authors, but I’m unaware of other possibilities to welcomingly address the issue, especially that reviewers generally are extremely interested in increasing their knowledge level themselves.

16 Likes

I'm going to move this to Evolution > Discussion, because I think it's more appropriate there; Site Feedback is for reporting technical / usability issues with the forums.

Thank you, I wasn't sure on the best category.

For a while I used to write a lot of specifications for software that built on to existing systems. The template we used had a "Pre-existing functionality" section early in the spec that talked about where we were and how did we get here before explaining what to attach on to the system.

It seemed to help to make sure that people who did not have institutional memory knew what to ask. The other advantage was that if this section was discussing things that were not really connected to the project at hand you could tell that the spec author was missing something important and reorient them (or me in that case.)

A big advantage to making people write this section first (both in the document and in reality) was that they had to run down some of the information themselves and fill in any gaps in their own understanding before they wrote the sections where they added functionality.

"Pre-existing functionality" was not that great of a name, but we tried "Background" and people skipped reading it because they assumed that they already knew it, which was a problem. I suppose there is some risk that "Prior Knowledge" might have the same difficulty, where people assume that it's impossible for them to be ignorant and just plunge in, but I think it's pretty good.

What about replacing the "Motivation" section in the existing template with the "Prior Knowledge" section? I feel like the "Prior Knowledge" should be early in the body of the pitch, because it forms the background for the rest of it. And the motivation for the project can go in the initial part of the pitch (the elevator paragraph). The detailed motivation can go in the real body of the pitch.

There are two components of Prior Knowledge I think:

  1. Things that people have already done or already know as shared knowledge on the project.
  2. Things that are background CS/Math knowledge that some people probably know in depth and some people probably don't, just because of, say, the specific classes you took or did not take in school.

In the first category, if you pitch something that reduces to adding Either you can reference other discussions about that and then explain why you're doing something different.

In the second category, if you want to do maybe generalized abstract data types you'd link up to definitions and examples in other languages. The two categories might be mixed together for some pitches.

There may be other categories I have not thought of, or those might not be the right categories, I'm not sure. I think if we update the pitch template we probably need to provide some really concrete guidance on what goes in this section so people complete it. I was thinking about the categories because I was trying to come up with a couple of sentences that would go in the template, but I'm not sure I have quite reached that. What do other people think?

2 Likes

I feel that "Motivation" and "Prior knowledge" are simply orthogonal in their goal: "Motivation" tries to make an argument, while "Prior knowledge" would try to be sort of a broader reference that does not necessarily discuss the pros and cons of things being referenced and doesn't even necessarily have to be tightly connected to the proposed change. Yes, they're both introductory sections, but in a different manner.

Yeah, having such a section does not ensure that it will be consulted with. We could add a note to the "what goes into a review" preamble, but even if people don't read it, it would still provide an opportunity for much milder policing ("make sure you read the 'Prior knowledge' section" vs. "make sure you're educated"). Anyways, any other section has the same potential of not being read, so shrug :man_shrugging:t3:

I particularly thought that this would be quite low-effort for proposal authors because they, having done their own research, are very likely to straight have a browser tab open with a resource they've consulted. The very general guideline could be to "include resources you relied on when writing the proposal". We would need to make sure that the section is not too broad, i.e., such section in a proposal on coroutines should not merely say "make sure you are familiar with coroutines", but perhaps link to John's talk on coroutines I very much enjoyed watching.

Which is particularly my wish for proposals that are not of the Either kind of level, where any reasonable Swift developer could be expected to imagine the implementation of the feature, but of the level that makes an intrusive change to the core of the language and would require a lot of runtime facilities, while the proposal itself would prefer to not dive into such implementation details. Especially that some of the recent proposals are still "awaiting implementation" I couldn't help asking myself "ok, but how is this going to work".

4 Likes

To be honest, I'm would rather encourage folks to participate, rather than discourage folks from participating until they've consulted all these resources. There are already a lot of really competent Swift programmers who feel like they can't participate even though they do have sufficient knowledge (I was one of them until recently!). I also don't want to discourage folks from asking questions about the proposal because they feel like they have to go find the answer on their own in a sea of resources.

I understand the intention here, but I don't think this will prevent folks from commenting without reading the resources. I don't think it's a bad idea to provide extra resources if they exist, but calling them "prior knowledge" sounds like gatekeeping to me.

I think you're underestimating the amount of work this would add to writing a proposal. Many proposal authors are compiler contributors, and the knowledge they relied on to write their proposal is their understanding of the compiler implementation. There is no resource for that, and I certainly don't want the compiler source code to become "prior knowledge" for evolution reviewers.

15 Likes

@hborla I'm so very glad to see you responding in this discussion and to be able to talk to you on this, I actually was wondering if it would be a good idea to tag you here given your diversity effort.

Yes, that might have just been some unfortunate wording on my side. I'm not sure if you're familiar with the original discussion I was referencing, but my proposal is actually about preventing (another kind of) gatekeeping that occurred back then (admittedly, I just didn't want to use this word to not accuse anyone unnecessarily). Let me elaborate on that a bit more.


In the ideal world, that would be possible, but that wasn't what I or the people who actually did become accused of not being competent enough, experienced (I certainly wasn't feeling comfortable reading that). I stated that I understand both sides, so thus I understand both the uncomfortable feeling a very experienced compiler engineer might have had when encountering a comment where they feel disconnected with the comment's objections and thus having to explain the terms they might find obvious and find this annoying, as well as the commentee's feeling of being publicly accused of not being competent enough to at all participate in the discussion, even if they actively seeked to become more educated on the topic.

My proposal was about decreasing this kind of friction preemptively. More precisely, it was about reducing the possible annoyance factors for the both parties by providing a way where the parties could exchange external information without direct interaction that would cost time. I'm terribly sorry if it doesn't read that way.

Perhaps I do, but I don't also suggest that the authors go out of their way to explain the topic, as, again, my idea was to preemptively reduce the amount of explanation they would need to provide. Perhaps that lies again on the poor wording; if there is a different way to provide the context for a proposal in more inclusive terms, I'm all in.

To perhaps be able to talk in more compassionate terms: I'm also one of them! The ultimate aim is for the proposal review process to become as inclusive as possible.

1 Like

To me, this sounds like it's partially a moderation problem. With this proposed solution, it still seems like the burden is on the person who is being accused to defend themselves by saying they did read the preliminary resources. I'm not sure this solution will prevent people from still assuming the person doesn't know what they're talking about. In my experience, simply stating that I have read X, Y, and Z has not been helpful in convincing somebody who assumes that I'm incompetent.

One thing I've been thinking about is whether there should be a parallel thread for evolution reviews that is dedicated to questions about/understanding the proposal, rather than evaluating or giving your opinion on it. I'm not sure whether that's a good idea, but at least nobody can argue that people shouldn't be participating because they don't understand - the entire point of the parallel thread would be to help people understand what's being proposed and how they will use it. This also seems like a lower barrier to entry to participating in evolution. Another idea that has come up is to change the evaluation template to make it clear that questions and such are welcome.

17 Likes

How about a proposal still trying to provide some complementary material? Such section could — aptly — be called "complementary material". That would possibly solve the issue of it being worded as "required" for both the side of the author, in having to provide it, and the side of a reviewer, in being required to read it. Just genuinely pointing at some things that would make it better for reviewers to understand the intention, instead of "teaching" them, which could sound patronising.

I would be fully into having a parallel thread like that. Maybe not by default (or maybe?), but it could be spawned alongside the original one if the community feels that we could benefint from more clarification not necessarily leading to evaluation of the proposal. I'm wondering if that would make it a "pen for outsiders" though.

I do think this is a good idea in general (although I don't think it's a solution to the problem we're talking about). I did something similar in the SE-0293 proposal - there's an appendix because the proposal talks about some confusing mutability rules (that already exist) for property wrappers. It wasn't documented anywhere though, so I wrote the explanation myself in the proposal.

Yeah, this is a fair concern. I guess it depends on how it actually plays out in practice. Part of the reason why I'm not sure it's a good idea is because I could totally see the "questions" thread getting a lot more activity than the actual review thread, so we've just moved all of the review activity from one place to another. It also seems like it would raise the barrier for participating on the review thread itself (e.g. people might feel like they have to completely understand the proposal before commenting on the review thread).

4 Likes

I was thinking that adding a set of resources early in the specification would encourage people to participate in the sense that they'd be able to click on them and get oriented. Maybe the word "knowledge" is signaling that you have to know this beforehand and otherwise you should not be here, which is gatekeepery. The background stuff should help people understand the rest of the pitch without necessarily causing deep research insecurity.

I guess one example/counterexample is from GADTs, where I think the pitch points at something on HaskellWiki (sorry if I am thinking of the wrong thing, might have followed the link from the Wikipedia page myself) that is not that easy to follow even if you like Haskell. That's helpful for some people but not for everybody in the community. (Again I did not just reopen the pitch so I might be thinking of something else, but the example is easy to replicate with all kinds of other things.)

Maybe the guidance should be that the background should be generally accessible to Swift programmers, so you could do links to Swift code, Wikipedia definitions, maybe blog posts by Swift programmers, possibly math blog posts if they don't require professional mathematical training (graduate degrees in math I mean), something like that.

When I used to do this, it was significant work to put together the pre-existing part. Most of the work was work I had to do anyway because I had to know what was going on and so on, and at least part of the motivation from the perspective of my boss was to make people on my team do some of the background stuff ourselves so we were not putting it on the people in the next stage of the product, and also so the body of the specification was more accurate. I think in @wtedst 's idea, you have a lot of tabs open while you're writing the pitch so you can kind of grab the URLs.

I used to write fairly short sentences with references to maybe functions in source code, outside requirements documentation, or for instance things that customers had reported. I had already looked at this as part of figuring things out, so if I remembered to make notes it was not that much work to add the reference, even if it was to a file on a shared server or a CRM ticket or something.

My point was that it was significant work but not a huge burden, I might have lost track of that.

The compiler implementation as background knowledge might deserve a separate topic. I do think that it would be helpful in this section we are mooting to include information about how and why things are implemented in the compiler, even if it's only explanatory, if the person who is writing the pitch knows about it. That might still be pretty short, it could be like "<person 1> and I talked about this in 2017, we decided to do it X way, you can see some comments about it in this commit". That would point people into the compiler code if they wanted to go there. I'm not sure if it might drive away people who are not comfortable clicking into a commit of C++ code though. But it is important to have that background there.

I'm feeling like the prior knowledge section should be very explicitly helpful, in order to prevent gatekeeping or the "I can't possibly explain this, I don't know what language this is in" effect. Probably this needs some rules or guidance in how to do that. I'm kind of reaching toward this as I write this.

The prior-knowledge solution will not prevent this. It can help at the meta level but at the community level it might be good to have some way to signal that the disrespecter needs to adjust their understanding of what's happening. I have seen a bunch of this face-to-face as well as in the computer, and also have seen some differential outcomes when I start talking out of ignorance on the forum.

Up to a point having background that people can go to should help up to a point but it also can add ammunition when people want to disrespect, for instance if somebody reads the background material but does not have the fluency with it that someone else has, then inadvertently transposes two terms (says "existential type" rather than "opaque type" or whatever) and then somebody pounces along the lines of "you just learned that today."

Writing this down I'm feeling differently than I did before, maybe because I'm thinking through a bunch of gatekeeping and disrespect scenarios.

People should be participating in evolution even if they don't understand pitches completely. The burden of explanation is on the person who wrote the pitch and if people don't understand it they should be able to ask for an explanation.

Some of that is a forum-tone or moderation thing, but also if the pitches are too hard to understand then people will be intimidated intrinsically and then get further intimidated by people who are fluent in some of the concepts and can talk about them in a way that can exclude people who are just working through the ideas.

I was thinking that the prior-knowledge/background section would help with this by bringing people who were relatively new to the level of detail in the pitch up to speed on the concepts, but I'm less convinced of that now having thought it through.

As a process thing I think adding a parallel thread makes things a little more complicated, because people will lose track of where to post and so on, and I really think that I-don't-understand-this is good pitch feedback on the regular evaluation thread. The places where people don't understand are valuable in revising the pitch.

I went a little long on this and combined responses to two posts. I don't have a complete resolution obviously, we should discuss all of this further.

4 Likes

Even as I've been following the majority of ongoing discussions, I can't figure out the context of the "issue" you're referring to and can't speak to what's going on, so I have to go by your comments in this thread.

Swift Evolution is specifically for user-facing changes to the language, allowing members of the community who are users to weigh in on how those changes would impact them. This provides instantly an answer to one of the most important questions when writing any text: Who is the target audience I'm writing for?

Authors of proposals should try to write in such a way that the greatest number of possible users of their proposed feature (not necessarily users of Swift generally) could give useful feedback about how any user-facing changes would affect them. Any text that is necessary to accomplish that goal should be included, and any text that's not necessary to accomplish that goal should be excluded.

The details of how a feature is to be implemented obviously dovetail with how the user-facing aspects are designed, but proposal authors don't have to explain their implementations beyond what's necessary to understand the user-facing design itself. Indeed, although an implementation is now required before a proposal proceeds to review, further iterations of the implementation occur after acceptance; in fact, there's no requirement that even a single line of code from the original implementation remain a part of the implementation that's ultimately merged.

Implementations of many features are going to be necessarily more complicated than the features themselves. Adding text, even links, about the implementation when such knowledge isn't necessary to understand the proposal is going to alienate users who, as @hborla describes, are fully competent Swift programmers and would likely have useful comments to make, but who may not be up on compiler implementation trivia and who don't have to be.

On the flip side, while educating others on how to implement a compiler feature is certainly useful and worthwhile, I think it's not appropriate to pile this on proposal authors. Implementers are expected to engage with code reviewers about what they want to merge; expecting implementers additionally to field informational questions from folks with varying familiarity with compiler internals is something that they haven't signed up for.

In general, better communication is about meeting each other in the middle, requiring an effort on both sides. Adding requirements produces more communication, but not necessarily better communication: the only thing for certain is that it makes the journey longer on both sides, making it harder and not easier to meet each other in the middle.

One addition to Swift proposals that I've advocated for previously is to bring in a section titled "How do we teach this?" (which I believe was added to Rust RFCs, or another "modern" programming language's analogous procedure). Obviously, such a section should spell out in sufficient detail how to use the proposed feature for a programmer otherwise generally familiar with Swift; part of the process of reviewing a feature would be explicitly evaluating whether it's sufficiently "teachable," and questions about understanding the feature would serve a double purpose during review of clarifying the proposal text and improving a feature's teachability.

15 Likes

I agree with this, the second quote in particular.

I'm looking at "Proposed Solution" and "Detailed Design" in the existing template at 0000-swift-template.md. Neither of those sections really covers the teachability issue based on their descriptions. "Proposed Solution" is close but the focus is slightly different.

It might be kind of costly to add a new section to the template though, especially if it duplicates in part an existing section. Does it make sense to adjust the guidance for "Proposed Solution" to try to focus it on teachability?

Actually writing the proposed solution in a way that is clear and teachable is probably more work than a lot of the other stuff we have talked about. In the discussion phase it's easy to write a lot of words and just to keep adding as people discuss.

To make it teachable you have to really shrink it down. You can compare how short the sections are in The Swift Programming Language. I assume they got that way by being rewritten over and over again, which took a lot of time and effort.

I'm not sure how you'd write guidance that said to rewrite the section over and over again aiming to make it concise though.

I feel like the "Proposed Solution" section should be structured and concise enough that it tells a Swift programmer what they need to know to use the feature. That might also be something to add to the guidance.

Then the guidance for "Detailed Design" could reflect things that a working Swift programmer
does not have to bring to the front of their mind while using the feature, even if they know about it.

Adding isPower(of:) to BinaryInteger is a pretty good example. The "Proposed Solution" section is how somebody would use this and what gets returned and when. The "Detailed Design" section talks about the fast and slow paths and details for different cases.

1 Like

I think this should be the goal of Detailed Design. This sentence in the template seems to place focus in the wrong area:

The detail in this section should be sufficient for someone who is not one of the authors to be able to reasonably implement the feature.

I mean, sure, other compiler engineers should be able to figure out how to implement the feature, but IMO that's not really the purpose of Detailed Design from the reviewers standpoint. In my opinion, the purpose of Detailed Design is to describe the syntax and semantics of the feature for users of the feature. For example, this is exactly what the Detailed Design of SE-0258 does. Of course, it talks about compiler-synthesized properties, but that's a fundamental part of the property wrapper mental model for users.

Maybe one of the questions in the review template should specifically ask something along the lines of "what questions do you have about the proposed feature?". I'd want to be careful about phrasing to avoid reviewers feeling self conscious about their own learning process. Like, if the question were "how easy was it for you to understand the feature", someone might feel really bad about saying it was hard for them to understand.

8 Likes

I think this would be helpful, and I'd prefer this over having two parallel threads per review. Questions are valuable feedback in that they highlight which areas are unclear or under-developed, so I'd be wary about relegating them to a separate thread.

3 Likes

I can agree with that only to a certain extent. Of course I wouldn't expect authors to discuss, dunno, the lines of code added to the typechecker, but as soon as code generation is involved, things change quite a bit. Perhaps that's how I personally learn or use the language, but I would be interested in at least the high-level details on how protocol witness tables work if I were to review a proposal that adds existential types to the language because this affects performance — and I view (the high-level aspect of) performance as part of user-facing design. I'm obviously not sure how a proposal would talk about that without authors needing to write a separate book, so I resorted to the least intense suggestion of "just link to something" if that something exists, even if it's a bit unapproachable.

But I recognize that this might be alienating and defeat the purpose. I would prefer such information to be included nevertheless, but the understanding of such should definitely worded as non-binding. Still, if I as a reviewer do happen to understand that in the end, that might contribute to me being able to produce much more targeted discussion, which was part of the "issue" I decided to only vaguely refer to for the sake of not disclosing any identities.

This passage from @hborla reiterates on my idea and how I would like to see it.

Putting aside that existential types already exist in the language, I disagree with this completely: Protocol witness tables are an implementation detail that shouldn't ever need to be mentioned in any Swift Evolution proposal for readers to understand a user-facing feature; otherwise, protocol witness tables would cease to be only an implementation detail.

And by the same token, authors of a proposal shouldn't ever feel obligated to write up anything about compiler internals. In fact, I would go so far as to say that if a feature can't be explained without surfacing a compiler-internal concept, then that should be a red flag to the author that the feature's design is leaking internal implementation details into the user-facing model that it shouldn't, and it should be rethought.

5 Likes

Right, I'm using existential types as an example of an already existing feature that I can already look into the implementation of to discuss it less theoretically, but that could be anything.

My perception of high-level languages is different though. The extremely user-facing observation that (an operation on) [Int] is much faster than [Intable] (for a theoretic Intable) still unavoidably leaks something about their implementation, regardless of how much we want it to be it an implementation detail merely. Users do not write code for abstract machines, they write code for very concrete CPUs that have cache lines of precise widths, and the way I regard high-level languages is as a tool that "just" helps me not write assembly, not as a tool which makes assembly non-existent.

That is, if I were to encounter a proposal which adds some runtime facilities, be they intended as implementation details or not, I would want to know about them. I, as a user, would want to be able to assess how much potentially this affects the performance of my code, what tradeoffs there are etc. — perhaps just on the level of Big-O — because that would help me make the decision whether I even want such modification to the language. An abstract talk about existential types wouldn't help me in that assessment; I would want to know what actually would be generated by the compiler and ultimately run on my CPU — again, in broad strokes, but still.

The Swift Evolution process does not govern all aspects of the Swift open source project, not even all aspects that would affect you as a user. Internal changes are made every day to the Swift compiler which affect the performance or even behavior of your code that aren't subject to Swift Evolution. For example, changing the encoding of native strings from UTF-16 to UTF-8 was a major overhaul simply announced after it was complete, while changing the sorting algorithm and the floating-point-to-string conversion algorithm were done without any fanfare.

By the same token, while users are perfectly justified in taking an interest in all aspects of a proposed feature, the review prompts specifically solicit your opinion of what's in the proposal. As a reviewer, you are not asked your opinion on details of implementation tradeoffs. That is not to say that others (such as the core team, or the code owners) won't consider those issues, just that it's not part of the reviewer's job in the Swift Evolution process.

4 Likes

Totally agree. That said, proposals do already link to the implementation PR, and anybody is welcome to give it a read, ask questions, and discuss implementation tradeoffs there. @wtedst I would encourage you to get more involved in code review given your interest in how features are implemented!

2 Likes
Terms of Service

Privacy Policy

Cookie Policy