Skip to content

[tools] Ignore comments in federated safety check #4028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

stuartmorgan-g
Copy link
Contributor

Because we treat analysis warnings as errors, there are common cases where non-breaking changes to part of a federated plugin (adding an enum value, deprecating a method, etc.) are CI-breaking for us. Currently we can't ignore those issues in the same PR where they are added, because doing so would violate the federated saftey check, adding extra complexity to those PRs.

This improves the change detection logic for the federated safety check to ignore comment-only changes in Dart code, which should allow us to add temporary // ignore:s in the PRs that create the need for them.

Fixes flutter/flutter#122390

Because we treat analysis warnings as errors, there are common cases
where non-breaking changes to part of a federated plugin (adding an enum
value, deprecating a method, etc.) are CI-breaking for us. Currently we
can't ignore those issues in the same PR where they are added, because
doing so would violate the federated saftey check, adding extra
complexity to those PRs.

This improves the change detection logic for the federated safety check
to ignore comment-only changes in Dart code, which should allow us to
add temporary `// ignore:`s in the PRs that create the need for them.

Fixes flutter/flutter#122390
@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label May 17, 2023
@auto-submit auto-submit bot merged commit a78a913 into flutter:main May 17, 2023
@stuartmorgan-g stuartmorgan-g deleted the federated-safety-ignore-comments branch May 17, 2023 20:28
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 18, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 18, 2023
flutter/packages@5c69914...b31a128

2023-05-18 stuartmorgan@google.com [flutter_plugin_android_lifecycle] Fix lints (flutter/packages#4030)
2023-05-18 ian@hixie.ch [rfw] Fix a typo in the API documentation (flutter/packages#4023)
2023-05-18 ditman@gmail.com [ci] Manual flutter roll (flutter/packages#4033)
2023-05-17 4195507+fmt-Println-MKO@users.noreply.github.com [flutter_adaptive_scaffold] exchange BottomNavigationBar with NavigationBar (flutter/packages#3746)
2023-05-17 stuartmorgan@google.com [tools] Ignore comments in federated safety check (flutter/packages#4028)
2023-05-17 reidbaker@google.com Revert "[url_launcher] Set broadcast reciever visability as required by target api 34" (flutter/packages#4027)
2023-05-17 stuartmorgan@google.com [camera] Fix Java lints in camerax version (flutter/packages#3966)
2023-05-17 34871572+gmackall@users.noreply.github.com [image_picker] Upgrade androidx.activity to 1.7.0 and add a dependency on kotlin-bom to align kotlin transitive dependencies (flutter/packages#3968)
2023-05-17 49699333+dependabot[bot]@users.noreply.github.com [espresso]: Bump com.squareup.okhttp3:okhttp from 4.10.0 to 4.11.0 in /packages/espresso/android (flutter/packages#3804)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
flutter/packages@5c69914...b31a128

2023-05-18 stuartmorgan@google.com [flutter_plugin_android_lifecycle] Fix lints (flutter/packages#4030)
2023-05-18 ian@hixie.ch [rfw] Fix a typo in the API documentation (flutter/packages#4023)
2023-05-18 ditman@gmail.com [ci] Manual flutter roll (flutter/packages#4033)
2023-05-17 4195507+fmt-Println-MKO@users.noreply.github.com [flutter_adaptive_scaffold] exchange BottomNavigationBar with NavigationBar (flutter/packages#3746)
2023-05-17 stuartmorgan@google.com [tools] Ignore comments in federated safety check (flutter/packages#4028)
2023-05-17 reidbaker@google.com Revert "[url_launcher] Set broadcast reciever visability as required by target api 34" (flutter/packages#4027)
2023-05-17 stuartmorgan@google.com [camera] Fix Java lints in camerax version (flutter/packages#3966)
2023-05-17 34871572+gmackall@users.noreply.github.com [image_picker] Upgrade androidx.activity to 1.7.0 and add a dependency on kotlin-bom to align kotlin transitive dependencies (flutter/packages#3968)
2023-05-17 49699333+dependabot[bot]@users.noreply.github.com [espresso]: Bump com.squareup.okhttp3:okhttp from 4.10.0 to 4.11.0 in /packages/espresso/android (flutter/packages#3804)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
Because we treat analysis warnings as errors, there are common cases where non-breaking changes to part of a federated plugin (adding an enum value, deprecating a method, etc.) are CI-breaking for us. Currently we can't ignore those issues in the same PR where they are added, because doing so would violate the federated saftey check, adding extra complexity to those PRs.

This improves the change detection logic for the federated safety check to ignore comment-only changes in Dart code, which should allow us to add temporary `// ignore:`s in the PRs that create the need for them.

Fixes flutter/flutter#122390
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[packages] Do line-level diffs for federated safety checks
2 participants