From 13f14c78240846eabb972e9e93b8a49536339bd4 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Mon, 9 Sep 2024 19:06:21 +0000 Subject: [PATCH 1/3] Remove references to implementation plan documents. We haven't created implementation plan documents for years. We decided that for now, it's better to document the process we're actually following. --- doc/life_of_a_language_feature.md | 37 +++++++++---------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/doc/life_of_a_language_feature.md b/doc/life_of_a_language_feature.md index 0419665022..2209a57b8b 100644 --- a/doc/life_of_a_language_feature.md +++ b/doc/life_of_a_language_feature.md @@ -41,6 +41,10 @@ As mentioned, alternative proposals should have their own issue and writeup. All proposals should be linked from and link to the 'request' for the user problem/feature request they are trying to address. +For smaller and non-controversial features, we will sometimes skip this step and +proceed directly to the [Acceptance and +implementation](#acceptance-and-implementation) phase. + ### External (outside of the language team) feedback We expect to use the github issue tracker as the primary place for accepting @@ -55,40 +59,23 @@ significant changes. If consensus is reached on a specific proposal and we decide to accept it, a member of the language team will be chosen to shepherd the implementation. -The implementation will be tracked via three artifacts: - - - An implementation plan document +The implementation will be tracked via two artifacts: - A 'implementation' issue. - A feature specification document -The implementation plan must be located in a sub-directory with the name of the -feature (`/[feature-name]/`) located inside the `accepted/future-releases/` -folder. The filename should be `implementation-plan.md`. The implementation -plan should generally include at least: - - - Affected implementation teams. - - - Schedule and release milestones. - - - Release flag if required, plan for shipping. - The 'feature specification' is **a single canonical writeup of the language feature** which is to serve as the implementation reference. Feature specifications use Markdown format. The file name should be -`feature-specification.md`, and the feature specification should -be located in the same sub-directory as the implementation plan. - -A meta-issue (labelled `implementation`) will be filed in the language -repository for tracking the implementation process. This top of this issue must -contain links to: - - - The related `request` issue +`feature-specification.md`, and the feature specification should be located +inside the `accepted/future-releases/` folder. - - The related `feature` issue +The implementation issue (labelled `implementation`) will be filed in the +language repository for tracking the implementation process. This top of this +issue must contain links to: - - A link to the implementation plan + - The related `request` and `feature` issues (if they exist) - A link to the feature specification @@ -145,11 +132,9 @@ shipped (e.g. `2.1`). 2.0/ 2.1/ super-mixins/ - implementation-plan.md feature-specification.md future-releases/ spread-operator/ - implementation-plan.md feature-specification.md /resources/ [various supporting documents and resources] From 30ccdb0ce66d5c5e2b20cf1b294c9b3fe1803779 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Mon, 9 Sep 2024 23:33:52 +0000 Subject: [PATCH 2/3] Additional life of a language feature documentation. This commit adds the following information: - Information about kick-off meetings - How to create implementation tracking issues - How to create a feature flag - How to do language testing - A note to remember to do Google3 testing (details of which will be described in a separate Google-internal document). --- doc/life_of_a_language_feature.md | 136 ++++++++++++++++++++++++++++-- 1 file changed, 130 insertions(+), 6 deletions(-) diff --git a/doc/life_of_a_language_feature.md b/doc/life_of_a_language_feature.md index 2209a57b8b..9dc0088d3f 100644 --- a/doc/life_of_a_language_feature.md +++ b/doc/life_of_a_language_feature.md @@ -79,20 +79,144 @@ issue must contain links to: - A link to the feature specification +### Kick-off meetings + The next step of the implementation issue is to get sign-off from all of the relevant implementation teams indicating that they understand the proposal, believe that it can reasonably be implemented, and feel that they have -sufficient details to proceed. There may be further iteration on the proposal +sufficient details to proceed. + +At a minimum, the set of relevant implementation teams should nearly always +include both the analyzer and CFE teams. Often, back-end teams (Dart native +runtime, Dart for web, and Wasm) should be included too. Note that if a feature +consists entirely of a new piece of syntactic sugar, it can be tempting to +assume that it's not necessary to consult with any back-end teams (since the CFE +will lower the new syntax into a kernel form that is already supported). But +this can be a dangerous assumption. It's easy to forget that even in features +that appear to consist purely of syntactic sugar, some back-end support may be +needed in order to properly support single-step debugging or hot reload. Also, +some work may need to be done on back-end code generators in order to make sure +that the new feature is lowered into a form that can be well optimized. To avoid +missing things, we prefer to err on the side of asking teams whether they're +affected, rather than assuming they won't be. + +Since feature specification documents are usually long and very detailed, we +like to begin this sign-off process with a set of kick-off meetings, typically +one for each affected implementation team. These meetings are on opportunity for +a language team representative to present a broad outline of the new feature, +give some concrete examples, and collect insights from the implementation teams +about what aspects of the implementation might need special attention. + +There may be further iteration on the proposal in this phase. This sign-off must be recorded in the implementation issue. Changes will be done in place on the writeup. -## Testing +### Implementation issues + +A typical outcome of the kick-off meetings will be a set of implementaiton +issues in the SDK repo, to track specific pieces of work that need to be +done. + +Additionally, Kevin Chisholm has a script for generating the core set of +implementation issues for a feature: +https://github.com/itsjustkevin/flutter_release_scripts/blob/main/languageFeatures.js. We +like to use this script for larger features. It creates a large number of +issues, though, so we will sometimes skip it for smaller features that only +require a small amount of work. + +Some teams have checklists that they consult when creating implementation +issues, to make sure important areas of work aren't forgotten. For example: + +- [Implementing a new language feature + (analyzer)](https://github.com/dart-lang/sdk/blob/main/pkg/analyzer/doc/process/new_language_feature.md) + +- [Implementing a new language feature (analysis + server)](https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server/doc/process/new_language_feature.md) + +### Feature flag + +Most new language features should be implemented using a feature flag. The +feature flag serves several purposes: + +- It allows the feature to be implemented over a series of CLs without + destabilizing the SDK. If it comes time to ship a new release of the SDK + before the feature is ready, the feature flag can remain off, so users won't + be exposed to a partially-implemented or buggy feature before it's ready. + +- It allows users to opt in to trying out features that have not yet been + released, by turning on the feature flag locally. + +- Once the feature is enabled, the feature flag logic ensures that it will only + affect users who have set their language version to a version that includes + the new feature. This is especially important for package developers who may + want to support a range of versions of the Dart SDK. + +Note that implementing a language feature using a feature flag is frequently +more work that implementing it without a feature flag, since the compiler and +analyzer must faithfully implement both the old and new behaviors. Occasionally +the language team may decide that the benefits of using a feature flag don't +justify this extra work. But this is a rare senario. If you are working on a +feature and believe you don't need to use a feature flag, please consult with +the language team to be sure. + +Creating the feature flag should be one of the first implementation +tasks. Here's how to do it: + +- Add an entry to `tools/experimental_features.yaml` describing the feature, in + the top section (above the line that says `Flags below this line are + shipped`). + +- Run `dart pkg/front_end/tool/fasta.dart generate-experimental-flags` to update + `pkg/_fe_analyzer_shared/lib/src/experiments/flags.dart` and + `pkg/front_end/lib/src/api_prototype/experimental_flags_generated.dart`. + +- Run `dart pkg/analyzer/tool/experiments/generate.dart` to update + `pkg/analyzer/lib/src/dart/analysis/experiments.g.dart`. + +- Add a static final declaration to the `Feature` class in + `pkg/analyzer/lib/dart/analysis/features.dart`. + +- Increment the value of `AnalysisDriver.DATA_VERSION` in + `pkg/analyzer/lib/src/dart/analysis/driver.dart`. + +- Example CL: https://dart-review.googlesource.com/c/sdk/+/365545 + +### Language testing The language team will generally write a preliminary set of language tests for a -feature. These tests are not intended to be exhaustive, but should illustrate -and exercise important and non-obvious features. Implementation teams are -encouraged to write additional language or unit tests. The language team may -also coordinate the writing of additional tests. +feature, in the SDK's `tests/language` subdirectory. These tests are not +intended to be exhaustive, but should illustrate and exercise important and +non-obvious features. Implementation teams are encouraged to write additional +language or unit tests. The language team may also coordinate the writing of +additional tests. + +An important use case for any new language feature is to ensure that the feature +isn't accidentally used by package authors that have not yet opted into a +language version that supports it. So in addition to testing that the new +language feature works when the feature flag is turned on, the tests added to +`tests/language` should verify that when the feature flag is turned off, the +previous behavior of the SDK is preserved. + +One important exception to preserving previous SDK behavior is that it's +permissible (and even encouraged) to improve error messages so that if the user +tries to use a disabled language feature, they receive an error explaining that +they need to enable it, rather than, say, a confusing set of parse errors. + +To enable the feature in a language test, include the line `// +SharedOptions=--enable-experiment=$FEATURE` near the top of the test (before the +first directive), where `$FEATURE` is replaced with the feature name. To disable +the feature in a language test, include the line `// @dart=$VERSION` near the +top of the test (before the first directive), where `$VERSION` is the current +stable release of Dart (and therefore is the largest version that is guaranteed +_not_ to contain the feature). + +### Google3 testing + +New features should be tested in Google's internal code base before they are +switched on. + +Details of how to do this will be described in a separate (Google internal) +document. ## Shipping From cc3e2888d14c27295fbf083f8863ab9c26fde86e Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Wed, 11 Sep 2024 21:29:35 +0000 Subject: [PATCH 3/3] Address code review comments --- doc/life_of_a_language_feature.md | 42 ++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/doc/life_of_a_language_feature.md b/doc/life_of_a_language_feature.md index 9dc0088d3f..06d6f64bc8 100644 --- a/doc/life_of_a_language_feature.md +++ b/doc/life_of_a_language_feature.md @@ -61,10 +61,10 @@ If consensus is reached on a specific proposal and we decide to accept it, a member of the language team will be chosen to shepherd the implementation. The implementation will be tracked via two artifacts: - - A 'implementation' issue. - - A feature specification document + - A 'implementation' issue. + The 'feature specification' is **a single canonical writeup of the language feature** which is to serve as the implementation reference. Feature specifications use Markdown format. The file name should be @@ -86,19 +86,31 @@ relevant implementation teams indicating that they understand the proposal, believe that it can reasonably be implemented, and feel that they have sufficient details to proceed. -At a minimum, the set of relevant implementation teams should nearly always -include both the analyzer and CFE teams. Often, back-end teams (Dart native -runtime, Dart for web, and Wasm) should be included too. Note that if a feature -consists entirely of a new piece of syntactic sugar, it can be tempting to -assume that it's not necessary to consult with any back-end teams (since the CFE -will lower the new syntax into a kernel form that is already supported). But -this can be a dangerous assumption. It's easy to forget that even in features -that appear to consist purely of syntactic sugar, some back-end support may be -needed in order to properly support single-step debugging or hot reload. Also, -some work may need to be done on back-end code generators in order to make sure -that the new feature is lowered into a form that can be well optimized. To avoid -missing things, we prefer to err on the side of asking teams whether they're -affected, rather than assuming they won't be. +Consider getting sign-off from the following teams: + +- The analyzer team. + +- The CFE team. + +- Back-end teams: + + - Dart native runtime + + - Dart for web + + - Wasm + +Note that if a feature consists entirely of a new piece of syntactic sugar, it +can be tempting to assume that it's not necessary to consult with any back-end +teams (since the CFE will lower the new syntax into a kernel form that is +already supported). But this can be a dangerous assumption. It's easy to forget +that even in features that appear to consist purely of syntactic sugar, some +back-end support may be needed in order to properly support single-step +debugging or hot reload. Also, some work may need to be done on back-end code +generators in order to make sure that the new feature is lowered into a form +that can be well optimized. To avoid missing things, we prefer to err on the +side of asking teams whether they're affected, rather than assuming they won't +be. Since feature specification documents are usually long and very detailed, we like to begin this sign-off process with a set of kick-off meetings, typically