API+1 bit for sdk/dart code?

[Jaeheon Yi]

Dear API Council,

The API Council Charter states that: 

"every change that modifies the Fuchsia API Surface must receive an API-Review+1" [1]. 

and presumably, libraries under sdk/dart are also part of the Fuchsia API Surface, like sdk/dart/fuchsia_scenic_flutter [2]. 

However, today it seems that modifications to these sdk/dart libraries don't actually require an API+1 bit in Gerrit. For example, see the recently submitted fxr/577314 [3]. 

Am I understanding the situation correctly? Is this mere oversight that can be remedied with a Gerrit fix? Has such a fix already been made?

Thank you,

Jaeheon

[1] https://fuchsia.dev/fuchsia-src/contribute/governance/api_council?hl=en#api_review

[2] https://fuchsia.googlesource.com/fuchsia/+/refs/heads/main/sdk/dart/fuchsia_scenic_flutter/

[3] https://fuchsia-review.googlesource.com/c/fuchsia/+/577314

[Adam Barth]

The API-Review flag in Gerrit is triggered when there's a ".api" file changed in the CL.  If you look at sdk_source_set:

https://fuchsia.googlesource.com/fuchsia/+/main/build/cpp/sdk_source_set.gni

... you'll see that we set the "api" and "api_contents" parameters for the sdk_atom:

      api = api_reference
      api_contents = sdk_header_files

These parameters cause the build system to generate and verify the ".api" files for that source set.  See the documentation for the sdk_atom template:

https://fuchsia.googlesource.com/fuchsia/+/main/build/sdk/sdk_atom.gni

Presumably we'd need to do something similar for Dart libraries in order to integrate them with the API-Review mechanism in the SDK pipeline.

Adam

--
You received this message because you are subscribed to the Google Groups "api-council" group.
To unsubscribe from this group and stop receiving emails from it, send an email to api-council...@fuchsia.dev.
To view this discussion on the web visit https://groups.google.com/a/fuchsia.dev/d/msgid/api-council/CADNd37in7%2BBZqoAxgxo%3DFTT-1YOMSVD_eA%2Bd_zhm3J2%3DHdaRoQ%40mail.gmail.com.

[Adam Barth]

+Misha Gridnev 

[Misha Gridnev]

This is basically https://fxbug.dev/6623. We tried to turn the dart API verification on a couple of times but had to roll it back because it was causing too much friction. One of the problems was dart being in //topaz repo at the time. Cross-repo API verification is notoriously painful. Topaz is no longer an issue since it was recently retired.

Perhaps we can turn the dart verification back on? +Jerry Belton any sharp corners we should watch out for?

[Jerry Belton]

fxr/592674 is currently WIP, but it will re-enable Dart API verification.

A few things have changed in the dart library code since I last tried to enable this, so I'll need some time to update things, but I hope to have things up for review in the next few days. 

[Jerry Belton]

fxr/592674 updates the Dart API diffing script, and submits .api files for all of the Dart SDKs in //sdk. 

A follow-up CL will enable the check at TOT.

[Jaeheon Yi]

Wonderful!!

Jaeheon

[Alice Neels]

This is great! Thank you for getting it in.

To view this discussion on the web visit https://groups.google.com/a/fuchsia.dev/d/msgid/api-council/CADNd37gcTDNUyhaND%2BbXWueNTvx1iBTvh4%3Du3vGvbVnnB-f1jA%40mail.gmail.com.

[Jaeheon Yi]

We're pretty close! https://fuchsia-review.googlesource.com/c/fuchsia/+/644269