Better organisation of reviews

Evolution Discussion
1

I'm a little disappointed by the very short length of swift-evolution reviews, and I'd like to suggest we do something about it.

In the last day, 4 reviews have started simultaneously:

Firstly: these reviews appear to have started within hours of each other, yet they all end on different days. Is there even a standard duration for evolution reviews?

Secondly: I don't like the pressure this puts me under. We're getting towards the end of the year, when people usually have deadlines or want to push to meet some kind of milestone. It's winter, which means people could easily be out for a week or more with a cold/flu (not to mention the pandemic, which is surging apparently everywhere). With the speed these proposals are moving at, even a week away from the forums could mean you miss some very important proposal (at least 2, possibly 3 of those proposals I would consider "high impact" and I definitely want to take a close look at - but I probably won't get time).

I don't think this kind of "blink and you'll miss it" approach to proposal reviews does this community any favours. Why do these reviews have to be so short?

This has been going on for a while, but so far it hasn't been too bad because the core team has been a little... relaxed about when they actually close a review (e.g. the review for SE-0290 was supposed to conclude on November 17th, but it's 2nd December and there is no announcement; presumably you can still leave feedback). Recently though, I've noticed some proposals really stick to their 7-day window, and it means I've missed the window because I couldn't find time for my actual life and job and to read, research and write a considered review on the proposal. When I did find time, I was literally finishing my response as the review closed before my eyes.

So what to do?

I'd like to propose a different structure: that we publish a monthly collection of proposals, and that those proposals run through the entire month. Any proposals which are ready for review within that time must wait until the next month's publication, along with any revised proposals.

This kind of structure would make it easier for all members of the community to schedule time to read, research and respond to evolution proposals, and give them each the time that they (and the Swift language) deserve. A month-long review might seem at first glance to be a massive expansion of the review period, but it is still a much shorter and simpler process than for other languages, and would not impact the speed with which these features are delivered in stable releases.

16 Likes
2

I definitely think something is amiss and that we need a remedy, but I'm guessing what you're proposing would not fit well with Apple's processes for Swift's Evolution, and adds potentially "unnecessary" delays in evolution proposals.

What I suggest is something a bit easier:

  1. a standard for review length be applied/published as part of Swift Evolution
  2. when a proposal is scheduled for review, make an announcement post with a link to the proposal so that people can have a heads up and get some extra time to review it, without feeling the pressure of people already discussing

I think the 2nd point has the side-effect benefit that those who read the proposal will have time to sit and think about it a little before their review can actually be posted - which might be a net positive on the quality of the first discussions on it.

1 Like
3

Just to explain what is happening (without defending it), there is more art than science here. I offered to take release management for SE-0293 yesterday and we felt like it should take ~1.5 weeks to run. I picked the end date to make sure it ran across two weekends to give people the chance to participate if they had a conflict at one or the other weekend.

I'm not sure exactly what happened with the other proposals, but some of them are smaller scope, so I assume the RMs decided that a shorter period was ok.

-Chris

5 Likes
4

It seems like that this pinpoints the possible difficulty/problem. No matter how small the scope, it will still take a certain amount of time period, so that sufficiently many people find the time to review. Sure the review will be quick, but accumulating reviewers has a minimum required timespan.

1 Like
5

(Speaking as myself, not for the Core Team as a whole)

Thanks for bringing this up.

I certainly like the idea of scheduling reviews in advance rather than launching immediately. A couple of days, at least, gives folks a heads-up that something is coming that they might be interested in, so it isn't a surprise when the review begins. If they wish to prepare their thoughts ahead of the review, they can.

However, I don't think that significantly extending the length of reviews is, in general, likely to produce different results. Most reviews only have a few days of active discussion before a consensus emerges---whether it's acceptance or for some set of substantive changes that warrant a second review. Especially with second and third reviews, we're generally reviewing a smaller slice of the proposal, so extra review time would mostly be an unnecessary delay.

Doug

9 Likes
6

Your proposal seems to ignore the needs of those authoring the proposals (and related Swift changes) and the constraints they work under. Forcing every proposal into monthly buckets would wreak havoc with the scheduling of work, especially when operating under a limited timeframe. I'm always a proponent for more openness around why particular proposals are chosen for review, when they'll be scheduled, and when they'll be accepted, but it doesn't seem feasible to force a particular timeline.

7

It might change the result, or it might not - I'm not sure it's fair to generalise about that. I've participated in reviews which seem to be going one way until somebody chimes in much later on with some domain-specific expertise or use-cases.

Personally, I'm finding it difficult to keep up, and that's why I want to revisit the question of whether it is really necessary for reviews to be so short. I feel that the onus should be on justifying why reviews are so short, rather than the opposite. With such short reviews, even the smallest of things can materially impact how much time somebody has to read, consider, and express their thoughts. Take the review about property wrappers as function arguments - something like that is likely to have a very big impact on the language, and it feels rather unprofessional that circumstances like a sick child would significantly limit how much somebody can participate in that review. It is mostly a question of professionalism.

As for the delay: the part of this that matters most (IMO) is whether the review time causes tangible delays in shipping the feature to developers in official releases. But official releases don't just spring up and surprise us: they are planned and staged, so I don't think this is actually going to delay when things ship. I can only remember one very time-sensitive proposal, and of course I expect the core team to reserve the right to make those kinds of exceptions as needed.

2 Likes
8

Proposing and implementing new features in a major programming language is not a small thing. It will affect many thousands of developers who use the language on a daily basis. I also feel that if somebody goes through all of that work, it's only respectful that it be given enough time for review.

Or to put it another way - I want to be able to give them more than my "gut feeling". But with time constraints being what they are, that's all I can afford to give them. I think they (and the language as a whole) deserves better than that, even if reviews are only advisory comments for the core team.

Maybe I'm just overworked (true) and nobody else feels this way, but I wanted to put it out there because I suspect it's not just me.

4 Likes
9

My point was that the engineers implementing these changes may also be overworked and have deadlines before the holidays and your proposal makes no acknowledgement of that fact. I don't think your proposal would help and would make the evolution process take even longer, which is not something the people working on the language would appreciate.

1 Like
10

Thanks for starting this discussion, @Karl—I've had similar feelings at times of being unable to devote the time to proposal review that I felt was deserved.

My subjective experience has been that there tend to be 'waves' of proposal reviews: periods of time where several proposals are under review at once, followed by periods where there is little-to-no review activity at all. IMO, it would be a better experience if these waves were avoided and overlapping review periods minimized.

Based on @Chris_Lattner3's post above, I'm struck by the implication that scheduling of individual proposals seems to be entirely delegated to each RM as they see fit (please correct me if I've misunderstood). Perhaps it would be better for review scheduling to be taken up more as a collective effort. I realize that this would put a larger coordination burden on the Core Team, but I think even a policy of something like "at least one week between the start times of consecutive reviews" would alleviate much of the frenetic feeling that sometimes arises.

Aside: a 'nice-to-have' would be for Swift Evolution to have some sort of 'calendar' view for the active/scheduled proposals, in addition to the current list view. :slightly_smiling_face:

Edit: clarified the suggested policy; "at least one week between consecutive reviews" -> "at least one week between the start times of consecutive reviews"

5 Likes
11

Sure, I guess many of the Apple engineers are trying to get these things closed before the new year. But so is everyone else! I don't see how it justifies a lightning-quick review.

My proposal wouldn't just make the review period longer (although it would do that); it would also make it more regular. Everybody would know when that month's round of reviews is going to show up, so you wouldn't have to stay glued to the forums for fear of missing something with a high impact.

2 Likes
12

I agree with Doug that I don’t think we’d actually get more feedback from longer review periods—we’d just slow down development. But that doesn’t mean Karl is wrong to feel like the review load is a little high right now. I noticed this too—it’s not so much that I couldn’t keep up, but it’s my job to keep up, and that’s not true for many Evolution participants.

@Karl I wonder: To what extent do you feel like this is too much because the review periods are too short, vs. because there are so many reviews starting at once? Three new proposals + a re-review on a single day does seem a little excessive to me, but I don’t think I’d feel that way if they were staggered over a couple of weeks.

If the problem is just too many reviews running at once, I could see us putting in place guidelines like:

  • Reviews should start on a Monday (probably actually “first workday of the week in the US”).
  • Limited scope re-reviews should be at least 7 days.
  • Other reviews should be at least 14 days.
  • No more than two limited scope re-reviews and two other reviews should start in each week.
  • These are just guidelines; the core team is expected to deviate when there is time pressure or another special circumstance (e.g. more than two related proposals which should be reviewed together).

Lots of room to argue about the specifics (maybe reviews should start on a Friday and end on a Monday so there’s an extra weekend in there?), but you get the idea.

7 Likes
13

Won't presume to speak for @Karl, but personally I feel that the ~concurrent~ reviews are the bigger issue. This period feels particularly crammed because we also have the numerous concurrency proposals undergoing their pitch phase. They may not be formal reviews, but they still contribute (for myself) to the feeling of a large amount of high-stakes activity.

Without getting into the weeds of the specifics too much, I would welcome a set of guidelines of this sort. I'd also maybe throw in something about a scheduling announcement that will go out 7 (or 5, or 3) days before a review commences.

2 Likes
14

It's the combination. I wouldn't necessarily say that short reviews are fine if there are less concurrent reviews or vice versa (as @tkrajacic pointed out, the effective, available time somebody has doesn't scale like that).

Your ideas do seem like they would lighten the load, though, so personally I would appreciate that. Every little helps.

I'd also be interested to hear about people working in larger teams of Swift developers - especially these days, perhaps it would be easier for them to schedule internal group discussions if reviews were slightly longer and happened in more regular intervals.

15

One thing is that with such short reviews, it's easy to miss them if you're on holiday or something. It's not unusual for people to take 7-14 day holidays.

Now, a lot of developers are used to working after-hours, at weekends, and checking the forums while technically "on holiday" probably isn't too uncommon. But that's not really a great thing - it's important for mental health and to prevent burnout that people actually get time to switch off.

I really feel that if we lengthened the review periods across the board, that it would be a lot easier to do that.

2 Likes
16

I don't think extending the standard review time past 14 days is feasible for scheduling reasons. A proposal scheduled now for 14 days would miss the branch date for the Swift 5.4 Spring release, even if the proposal was universally accepted as-is and fully implemented. If you start lining up month-long reviews (+ expected re-reviews) for, say, the dependency chain of Swift Concurrency proposals you'll almost certainly push yourself past the next branch date. And longer review periods makes for more overlapping reviews, which means more things to track at once. We trade one problem for another.

I maintain that the primary issue is the surprise. Right now, it's plausible that a two-week holiday could completely straddle a review period and you wouldn't know until you saw that the review was done. If we schedule reviews in advance and it conflicts with your holiday, you can effectively time-shift: write your review in advance, decide to check in once to post it, etc.

For example, with the set of reviews that just launched, the proposals were effectively ready before the U.S. Thanksgiving holiday. Had the Core Team posted the schedule before that holiday, there would be no surprise early this week, and folks with an interest could have chosen to read the proposals over the holiday (or not) as appropriate. Anyone who wanted to schedule a discussion with their colleagues would have been able to do so during the review period.

I checked in with the Core Team, and we're going to try to "pre-schedule" some of the Concurrency-related proposals to try to help here. These are the largest proposals going through the process in the near future, and there are several of them with dependencies. We're mindful that we need to start the review process, that some parts of Concurrency seem to be mostly settled while others are much more in flux, and that we have another holiday season coming up during which there will be no reviews. Here's what we're looking at:

  • Schedule async/await for December 8...22

  • Schedule "Objective-C Interoperability with Concurrency" for December 10...22

  • Schedule "Structured Concurrency" for January 5...19.

  • Schedule" Async Sequences" for January 5...19.

    Doug

10 Likes
17

Looking at the cutoff dates for the last few releases:

So we do tend to have quite a bit of time between releases. I think there is probably some leeway to extend reviews within that time (maybe extending all deadlines by 7 days - so 14 days for re-reviews, 21 days for longer reviews).

Perhaps, but with the schedule you posted (very informative BTW, thanks!), we're apparently looking at 5 concurrent reviews (!) come December 10. So shorter review periods do not seem to be leading to less overlapping reviews today, and the overall load is still very high.

18

Most of those 5 will be at the tail end and are unlikely to be active. Moreover, today we are crunched between two holiday periods. If we had 21-day reviews and wanted to avoid additional overlap now, we'd schedule those after the holiday break---and then either have 4 concurrently-running concurrency reviews in January (most of which are large, so that's not great) or be staging them out and not be able to start the second round until the end of January. There's also a third round (e.g., actors, then global actors) that would follow, which would then only start in middle or late February, and that historical "March 18th" branch date you cited becomes a very, very real constraint.

We would very much like to accommodate people's schedules and improve the review process, but there are many constraints on this. We'll see if pre-scheduling reviews and having slightly longer review periods can help.

Doug

2 Likes
19

I don't think that extending the review time is required in general, and I'm happy for RMs to decide their own timeline. I don't want to see a uniform review period of an entire month, because that is clearly overkill for almost all proposals. It's always easier to argue that more time is better (e.g. it allows more time for consideration and more people to participate) but there are downsides too (e.g. it can waste a lot of people's time and it makes the whole system of making changes less efficient). I have personally seen more reviews that I wish had closed earlier than later, because there is a lot of active discussion but no new substantive arguments.

From what you've written here it sounds like you're personally feeling a lot of time pressure at the moment. There have been times when I haven't been able to read the forums for weeks, or months, also. I think you have to accept that you won't always be able to participate in reviews, and trust that the people who do participate and the core team will make good decisions without you.

This would be the worst case for me, because I don't have much time during the week, but it just makes the point that you can never please everyone.

20

I disagree with this sentiment.

Sometimes one review spots something that no one else sees, and turns the tide.