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 [email protected] [flutter_plugin_android_lifecycle] Fix lints (flutter/packages#4030)
2023-05-18 [email protected] [rfw] Fix a typo in the API documentation (flutter/packages#4023)
2023-05-18 [email protected] [ci] Manual flutter roll (flutter/packages#4033)
2023-05-17 [email protected] [flutter_adaptive_scaffold] exchange BottomNavigationBar with NavigationBar (flutter/packages#3746)
2023-05-17 [email protected] [tools] Ignore comments in federated safety check (flutter/packages#4028)
2023-05-17 [email protected] Revert "[url_launcher] Set broadcast reciever visability as required by target api 34" (flutter/packages#4027)
2023-05-17 [email protected] [camera] Fix Java lints in camerax version (flutter/packages#3966)
2023-05-17 [email protected] [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 [email protected],[email protected] 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 [email protected] [flutter_plugin_android_lifecycle] Fix lints (flutter/packages#4030)
2023-05-18 [email protected] [rfw] Fix a typo in the API documentation (flutter/packages#4023)
2023-05-18 [email protected] [ci] Manual flutter roll (flutter/packages#4033)
2023-05-17 [email protected] [flutter_adaptive_scaffold] exchange BottomNavigationBar with NavigationBar (flutter/packages#3746)
2023-05-17 [email protected] [tools] Ignore comments in federated safety check (flutter/packages#4028)
2023-05-17 [email protected] Revert "[url_launcher] Set broadcast reciever visability as required by target api 34" (flutter/packages#4027)
2023-05-17 [email protected] [camera] Fix Java lints in camerax version (flutter/packages#3966)
2023-05-17 [email protected] [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 [email protected],[email protected] 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