Putting specification review into the launch process

939 views
Skip to first unread message

Jeffrey Yasskin

unread,
Nov 16, 2022, 10:50:08 PM11/16/22
to blink-api-ow...@chromium.org, Sam Goto, Chris Wilson
Hi Blink API owners,

As part of a postmortem [1] for a set of mistakes around the FedCM spec development and I2S, Chris and I committed [2] to looking into how we could make specification review a more-required part of the Chrome/Chromium launch process. Since you're the only people in a place to gate on spec quality right now, we wanted to see how you'd like the process to evolve.

* Why change anything? The problems happened because the team assumed specs were primarily useful for ensuring interoperability, and so deprioritized requests to flesh out certain details around Fetch. In fact, specs are also useful for communicating the details of designs to reviewers, including external reviewers who might catch things that could fall into a Google review team's blind spots. Sam felt that the best way to explain this to teams at scale was to incorporate it into the formal launch approvals.

* How do we avoid imposing extra work on you? We could ask spec mentors to do a full spec review and sign off before the I2S gets sent. We could also split the responsibility, and make the API owners responsible for checking that the spec is detailed enough to help interoperability, but make the security and privacy review teams responsible for checking that the spec explains enough for external reviewers to double-check the internal teams' conclusions. Or something else.

What do you think?

Thanks,
Jeffrey

[1] http://irm/i_x7cNnDfUjnFJhexSkuCA (Sorry, only accessible to Googlers) Issues centered around github.com/fedidcg/FedCM#320.
[2] https://crbug.com/1383331 (Sorry, also only Googlers)

Yoav Weiss

unread,
Nov 17, 2022, 9:58:20 PM11/17/22
to Jeffrey Yasskin, blink-api-ow...@chromium.org, Sam Goto, Chris Wilson
Hey Jeffrey!

I think that getting spec mentors to explicitly sign off on the quality of specs would be extremely useful. It would significantly increase the level of the specs that are reaching I2S, and would give API owners the confidence that the spec is in a reasonable shape. We can then poke around and verify.

Would that be something that spec mentors be up for? 

Cheers,
Yoav

--
You received this message because you are subscribed to the Google Groups "blink-api-owners-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-api-owners-d...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-api-owners-discuss/CANh-dXmL52Xp5%3DKjJk6Bnbbi97HK5ckz%3DRTDwMifo4L_ZcW8GA%40mail.gmail.com.

Jeffrey Yasskin

unread,
Nov 18, 2022, 12:41:00 AM11/18/22
to Yoav Weiss, Jeffrey Yasskin, blink-api-ow...@chromium.org, Sam Goto, Chris Wilson
We'll need to check with the spec mentors if y'all think we do want to gate the I2S on their review. 

Yoav Weiss

unread,
Nov 18, 2022, 1:50:00 AM11/18/22
to Jeffrey Yasskin, blink-api-ow...@chromium.org, Sam Goto, Chris Wilson
I would potentially gate the sending on the I2S on a spec mentor review - basically tell feature owners to wait for such a review before sending it.
I wonder what other folks think.

Daniel Bratell

unread,
Nov 23, 2022, 2:47:57 PM11/23/22
to Yoav Weiss, Jeffrey Yasskin, blink-api-ow...@chromium.org, Sam Goto, Chris Wilson

I typically rely on TAG to bring up obvious spec shortcomings in their review and only look deeper at specs if there is no TAG review or I'm otherwise interested.  (Since this one was in an area where I'm not very familiar I might have overlooked flaws anyway). For FedCM there was a closed TAG review where a closer scrutiny would have revealed the statement:

"We largely think it's going in the right direction. this is not a full endorsement of the current API or architecture as specified in the CG report. "

- which is kind of saying that they either found the spec lacking, or did not do a full review. That is something I think we should have noticed and I don't recall us commenting on it.

(Also, with the post mortem and the bug hidden I am sure I'm not fully understanding what has gone wrong)

/Daniel

Rick Byers

unread,
Nov 25, 2022, 9:03:18 PM11/25/22
to Daniel Bratell, Yoav Weiss, Jeffrey Yasskin, blink-api-ow...@chromium.org, Sam Goto, Chris Wilson
Thanks for bringing this up Jeffrey,
I'm not sure how much to worry about this specific incident as it seems the right things ultimately happened without any delay, but I'm supportive generally of placing more emphasis on the importance of spec quality via the launch process. I think we, as API owners, have not been very consistent on the spec quality bar we apply and I have heard feedback from some other web stakeholders who wish we'd hold a higher bar. I love the role that spec mentors play here and I think elevating it makes sense. At the same time though, I'm very wary of adding overhead for the majority of changes which are straightforward and don't involve substantial new spec text. Perhaps there's a compromise here where intents that involve a NEW specification document are required to answer a section on specification review?
 
Also I'm cautious about increasing the barriers for new folks (especially non-Googlers) contributing to evolving the platform. What if someone struggles to find anyone to take the time to do a thorough spec review, what is their path forward? Overall I think there's a tradeoff here between risking making more mistakes that need cleanup later and risking increasing the barrier to entry for folks outside the established expert group. Personally I believe the web today suffers more today from the latter problem (thinking of all the developers/businesses who depend on the web but feel they have no agency over it) than it does from the former, but I admit this is entirely subjective.

How about for intents referencing a new (or substantially overhauled) spec, we ask for a one-line spec maturity summary from someone who has done a review? We could link to our spec mentorship program as one such venue for getting that, and can internally say it's required for Googlers if we want. But for the majority (eg. small new features in an existing spec following it's established pattern/process) it would be N/A. 

In the specific case of FedCM I suspect I contributed to the problem by arguing for not blocking on some of the big outstanding questions and encouraging the team to have a bias for shipping and iterating. I'm happy to hear feedback (publicly or privately) on what I could have perhaps done better too.

Rick

Jeffrey Yasskin

unread,
Nov 30, 2022, 12:22:43 AM11/30/22
to Rick Byers, Daniel Bratell, Yoav Weiss, Jeffrey Yasskin, blink-api-ow...@chromium.org, Sam Goto, Chris Wilson
I think a rule that new (or substantially overhauled) specs need a spec maturity/quality review would at least be a good place to start, and we could scale up or down depending on how it goes. Sam, does that sound like it would be enough to fix the problems you see?

Rick, that particular email came too late for it to have caused any of the problems here. The core of the issue wound up being that, without the spec including its integration with Fetch, there were potentially "big outstanding questions" that external reviewers couldn't even know to raise. I think the API owners generally do a good job of discouraging "we haven't gotten enough reviews to discover all the issues, but we're shipping anyway" while still encouraging sensible uses of "we know the issues, and will ship anyway", but we haven't done a good enough job of explaining how a spec helps make the reviews better.

Jeffrey

Jeffrey Yasskin

unread,
Apr 28, 2023, 3:01:34 PM4/28/23
to Jeffrey Yasskin, Rick Byers, Daniel Bratell, Yoav Weiss, blink-api-ow...@chromium.org, Sam Goto, Chris Wilson
I've finally surveyed the spec mentors about doing this[1][2], and while everyone who's replied has been generally supportive, three related worries came up:

1. There's some general worry about asking individual spec mentors to hold the line on spec quality.
2. In particular, one person reported getting negative promo feedback because they noted technical issues with a specification.
3. There's also a preference to keep most of the discussion between the mentor and the feature team, rather than making the mentor route through the API owners.

I have a CL at https://chromium-review.googlesource.com/c/website/+/4453886 that both describes how spec mentors should review specs, and adds this review to the launch process in a way that I hope mostly avoids the concerns above. What do you think?

Jeffrey

Yoav Weiss

unread,
Apr 29, 2023, 3:24:05 AM4/29/23
to Jeffrey Yasskin, Rick Byers, Daniel Bratell, blink-api-ow...@chromium.org, Sam Goto, Chris Wilson
On Fri, Apr 28, 2023 at 5:01 PM Jeffrey Yasskin <jyas...@chromium.org> wrote:
I've finally surveyed the spec mentors about doing this[1][2], and while everyone who's replied has been generally supportive, three related worries came up:

1. There's some general worry about asking individual spec mentors to hold the line on spec quality.
2. In particular, one person reported getting negative promo feedback because they noted technical issues with a specification.

That sounds.. bad
 
3. There's also a preference to keep most of the discussion between the mentor and the feature team, rather than making the mentor route through the API owners.

I have a CL at https://chromium-review.googlesource.com/c/website/+/4453886 that both describes how spec mentors should review specs, and adds this review to the launch process in a way that I hope mostly avoids the concerns above. What do you think?

Is the plan to also add a checkbox to the intent template verifying that the spec was indeed reviewed?

Jeffrey Yasskin

unread,
May 8, 2023, 8:51:38 PM5/8/23
to Yoav Weiss, Jeffrey Yasskin, Rick Byers, Daniel Bratell, blink-api-ow...@chromium.org, Sam Goto, Chris Wilson
On Fri, Apr 28, 2023 at 8:24 PM 'Yoav Weiss' via blink-api-owners-discuss <blink-api-ow...@chromium.org> wrote:
On Fri, Apr 28, 2023 at 5:01 PM Jeffrey Yasskin <jyas...@chromium.org> wrote:
I've finally surveyed the spec mentors about doing this[1][2], and while everyone who's replied has been generally supportive, three related worries came up:

1. There's some general worry about asking individual spec mentors to hold the line on spec quality.
2. In particular, one person reported getting negative promo feedback because they noted technical issues with a specification.

That sounds.. bad

Yes. There are really two ways to address it: 1) Make sure teams are receptive to feedback and don't complain if that feedback results in delaying their launch. 2) Make it clear that spec mentor reviews are advisory, and it's the API owners who need to balance spec quality against our desire to move the web forward, so teams don't have a reason to be mad at the reviewer. I've been leaning toward (2) because it seems easier.

3. There's also a preference to keep most of the discussion between the mentor and the feature team, rather than making the mentor route through the API owners.

I have a CL at https://chromium-review.googlesource.com/c/website/+/4453886 that both describes how spec mentors should review specs, and adds this review to the launch process in a way that I hope mostly avoids the concerns above. What do you think?

Is the plan to also add a checkbox to the intent template verifying that the spec was indeed reviewed?

Once the new chromestatus-based launch process is turned on, we should create a launch gate for "a spec mentor reviewed this specification", with the spec mentors as a group owning the approval bit for that gate. The wording of that gate will be important, but we can fine-tune that after the direction is approved in this thread.

Thanks,
Jeffrey


Sam Goto

unread,
May 24, 2023, 5:14:19 PM5/24/23
to Jeffrey Yasskin, Sam Goto, Yoav Weiss, Rick Byers, Daniel Bratell, blink-api-ow...@chromium.org, Chris Wilson
+ my google email, which I check more consistently :)

On Mon, May 8, 2023 at 1:51 PM Jeffrey Yasskin <jyas...@chromium.org> wrote:
On Fri, Apr 28, 2023 at 8:24 PM 'Yoav Weiss' via blink-api-owners-discuss <blink-api-ow...@chromium.org> wrote:
On Fri, Apr 28, 2023 at 5:01 PM Jeffrey Yasskin <jyas...@chromium.org> wrote:
I've finally surveyed the spec mentors about doing this[1][2], and while everyone who's replied has been generally supportive, three related worries came up:

1. There's some general worry about asking individual spec mentors to hold the line on spec quality.
2. In particular, one person reported getting negative promo feedback because they noted technical issues with a specification.

That sounds.. bad

Yeah, this sounds terrible :(
 

Yes. There are really two ways to address it: 1) Make sure teams are receptive to feedback and don't complain if that feedback results in delaying their launch. 
2) Make it clear that spec mentor reviews are advisory, and it's the API owners who need to balance spec quality against our desire to move the web forward, so teams don't have a reason to be mad at the reviewer. I've been leaning toward (2) because it seems easier.

3. There's also a preference to keep most of the discussion between the mentor and the feature team, rather than making the mentor route through the API owners.

I have a CL at https://chromium-review.googlesource.com/c/website/+/4453886 that both describes how spec mentors should review specs, and adds this review to the launch process in a way that I hope mostly avoids the concerns above. What do you think?

Is the plan to also add a checkbox to the intent template verifying that the spec was indeed reviewed?

Once the new chromestatus-based launch process is turned on, we should create a launch gate for "a spec mentor reviewed this specification", with the spec mentors as a group owning the approval bit for that gate. The wording of that gate will be important, but we can fine-tune that after the direction is approved in this thread.

Thanks,
Jeffrey



On Tue, Nov 29, 2022 at 4:22 PM Jeffrey Yasskin <jyas...@chromium.org> wrote:
I think a rule that new (or substantially overhauled) specs need a spec maturity/quality review would at least be a good place to start, and we could scale up or down depending on how it goes. Sam, does that sound like it would be enough to fix the problems you see?

I think it would help, but it is not clear to me that it would fix the problem going forward.


Rick, that particular email came too late for it to have caused any of the problems here. The core of the issue wound up being that, without the spec including its integration with Fetch, there were potentially "big outstanding questions" that external reviewers couldn't even know to raise.

It is not clear to me that that's the case: my intuition is that anyone sufficiently interested in making an assessment could have done that without a full Fetch integration. Sure, we could have made things clearer and easier for an external review, but I don't fully buy that the Fetch integration in the specification is necessarily the only/best way to make a good privacy and security assessment.
Reply all
Reply to author
Forward
0 new messages