Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Prevent solo: true from being committed #51712

Merged
merged 3 commits into from
Mar 28, 2024
Merged

Prevent solo: true from being committed #51712

merged 3 commits into from
Mar 28, 2024

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Mar 27, 2024

At some point, we inherited a change from the repo-level analysis_options.yaml that allowed deprecated members to be used. We rely on that analyzer error to prevent solo: true from being committed in our tests (example of one that slipped recently).

This PR overrides the deprecated_member_use error to enable it inside the web engine.

@github-actions github-actions bot added the platform-web Code specifically for the web engine label Mar 27, 2024
eyebrowsoffire
eyebrowsoffire approved these changes Mar 28, 2024
View reviewed changes
Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 28, 2024
@auto-submit auto-submit bot merged commit 4e6692c into main Mar 28, 2024
@auto-submit auto-submit bot deleted the mdebbar-patch-2 branch March 28, 2024 19:31
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 28, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 28, 2024
@engine-flutter-autoroll
4e6692c8e Prevent solo: true from being committed (flutter/engine#5…
1ae70ea 
…1712)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 28, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 28, 2024
@engine-flutter-autoroll
4e6692c8e Prevent solo: true from being committed (flutter/engine#5…
3a57205 
…1712)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Mar 28, 2024
@engine-flutter-autoroll
Roll Flutter Engine from f71e5ad8586b to a396dc1a03a9 (3 revisions) (#…
89ea492 
…145928)

flutter/engine@f71e5ad...a396dc1

2024-03-28 jacksongardner@google.com [skwasm] Use text position affinity when calculating word boundaries. (flutter/engine#51753)
2024-03-28 mdebbar@google.com Prevent `solo: true` from being committed (flutter/engine#51712)
2024-03-28 skia-flutter-autoroll@skia.org Manual roll Dart SDK from 291217c1d399 to bb65648e20e2 (3 revisions) (flutter/engine#51755)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,rmistry@google.com,zra@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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@goderbauer
Copy link
Member

cc @matanlurey The solo: true issue is an unintended side effect of #50575.

This change here is negating the stated goal of #50575: Deprecations in the Dart SDK (or any other external package) will now break the tree again if the deprecated properties are used in web_ui.

@matanlurey
Copy link
Contributor

cc @matanlurey The solo: true issue is an unintended side effect of #50575.

This change here is negating the stated goal of #50575: Deprecations in the Dart SDK (or any other external package) will now break the tree again if the deprecated properties are used in web_ui.

I don't think this change should have landed, that is, I don't think the web team unilaterally should be deciding to turn on lints we've decided not to have on as part of team-wide policy, and I'd like to consider reverting this PR.

Fwiw I did work on @doNotSubmit, which we should be able to use soon-ish (it will have to become available in a published package:meta, and then that has to be used in package:test, so it could be a couple of weeks?). See https://dart-review.googlesource.com/c/sdk/+/360481.

yjbanov added a commit to yjbanov/engine that referenced this pull request Apr 2, 2024
@yjbanov
Revert "Prevent solo: true from being committed (flutter#51712)"
6c225d6 
This reverts commit 4e6692c.
zanderso added a commit that referenced this pull request Apr 3, 2024
@zanderso
Revert "Prevent solo: true from being committed (#51712)"
db71233 
This reverts commit 4e6692c.
@zanderso zanderso mentioned this pull request Apr 3, 2024
Merged
zanderso added a commit that referenced this pull request Apr 3, 2024
@zanderso
Revert "Prevent solo: true from being committed" (#51858)
0da1b2e 
Reverts #51712

Reason for revert: As discussed in
flutter/flutter#143312, and in
https://discord.com/channels/608014603317936148/1224499330824802375. I
apologize if this seems hasty and not fully justified, but it is
important that the engine repo follows policies like this to make it
easier for our various teams to work together.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eyebrowsoffire eyebrowsoffire eyebrowsoffire approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mdebbar @goderbauer @matanlurey @eyebrowsoffire