Amend package:web tweaks to allow package:web roll #6793

srujzs · 2024-05-24T21:01:36Z

dart-lang/web@7604578 adds a number of APIs in order to get a more consistent API surface. This includes disablePictureInPicture and disableRemotePlayback, which both take in a bool and not a JSBoolean. However, it can't be rolled into google3 as this package will now be broken due to the way extension type methods take a higher precedence over extension methods.

This CL aligns those methods with the package:web equivalent so that usages of these methods can be consistent. controlsList is also amended to take in a String so that if it's ever added to the Web IDL, it doesn't conflict with the package:web definition that will be added then.

dart-lang/web@7604578 adds a number of APIs in order to get a more consistent API surface. This includes disablePictureInPicture and disableRemotePlayback, which both take in a bool and not a JSBoolean. However, it can't be rolled into google3 as this package will now be broken due to the way extension type methods take a higher precedence over extension methods. This CL aligns those methods with the package:web equivalent so that usages of these methods can be consistent. controlsList is also amended to take in a String so that if it's ever added to the Web IDL, it doesn't conflict with the package:web definition that will be added then.

flutter-dashboard · 2024-05-24T21:01:38Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

srujzs · 2024-05-24T22:10:07Z

Hmm, the bot isn't happy with my versioning here. Should I be doing a 2.3.0+1/2.3.1 and update the pubspec as well?

ditman

Hmm, the bot isn't happy with my versioning here. Should I be doing a 2.3.0+1/2.3.1 and update the pubspec as well?

@srujzs Yes please, do it 2.3.1 (2.3.0+1 is not a valid pub version number, ask me how I know :P)

As for the tests, please check here, where we do something similar but for the integration tests of the package (I can help you running those locally if needed):

https://github.com/flutter/packages/blob/main/packages/video_player/video_player_web/example/integration_test/pkg_web_tweaks.dart

srujzs · 2024-05-28T16:43:42Z

Yes please, do it 2.3.1 (2.3.0+1 is not a valid pub version number, ask me how I know :P)

Done!

As for the tests, please check here, where we do something similar but for the integration tests of the package

Ah I forgot that this repo has two such files to tweak package:web. Done.

srujzs · 2024-05-29T18:16:23Z

Friendly ping. :)

ditman

Thanks for the fix, ship it!

…ackages#6793)

flutter/packages@31d3329...910fabb 2024-05-29 58529443+srujzs@users.noreply.github.com Amend package:web tweaks to allow package:web roll (flutter/packages#6793) 2024-05-29 matanlurey@users.noreply.github.com Migrate `video_player/android` from `SurfaceTexture`->`SurfaceProducer`. (flutter/packages#6456) 2024-05-29 joonas.kerttula@codemate.com [google_maps_flutter] Undeprecate BitmapDescriptor methods (flutter/packages#6832) 2024-05-29 matanlurey@users.noreply.github.com Migrate CameraX from SurfaceTexture to SurfaceProducer. (flutter/packages#6462) 2024-05-29 matanlurey@users.noreply.github.com Migrate `camera/android` from `SurfaceTexture`->`SurfaceProducer`. (flutter/packages#6461) 2024-05-29 katelovett@google.com [dynamic_layouts] Remove the dynamic_layouts package (flutter/packages#6830) 2024-05-29 43054281+camsim99@users.noreply.github.com [camerax] Add notes about Android permissions (flutter/packages#6741) 2024-05-29 43054281+camsim99@users.noreply.github.com [Re-land] Bump legacy all_packages project AGP version to 7.0.0, Gradle version to 7.0.2 (flutter/packages#6742) 2024-05-29 engine-flutter-autoroll@skia.org Roll Flutter from a1a33e6 to c85fa6a (20 revisions) (flutter/packages#6829) 2024-05-29 katelovett@google.com [rfw] Migrate deprecated doc references (flutter/packages#6744) 2024-05-29 katelovett@google.com [flutter_adaptive_scaffold] Migrate MaterialStateProperty to WidgetStateProperty (flutter/packages#6743) 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://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

dart-lang/web@7604578 adds a number of APIs in order to get a more consistent API surface. This includes disablePictureInPicture and disableRemotePlayback, which both take in a bool and not a JSBoolean. However, it can't be rolled into google3 as this package will now be broken due to the way extension type methods take a higher precedence over extension methods. This CL aligns those methods with the package:web equivalent so that usages of these methods can be consistent. controlsList is also amended to take in a String so that if it's ever added to the Web IDL, it doesn't conflict with the package:web definition that will be added then.

srujzs requested a review from ditman as a code owner May 24, 2024 21:01

github-actions bot added platform-web p: video_player labels May 24, 2024

srujzs added 2 commits May 24, 2024 14:20

Remove unused import

4c1cbf8

Add changelog entry

ce60bdf

ditman reviewed May 25, 2024

View reviewed changes

srujzs added 3 commits May 28, 2024 09:39

Change version to 2.3.1

ae3e3c6

Amend integration test tweaks

c8891fb

Add todos to helpers in integration_test

1284b2c

Merge remote-tracking branch 'upstream/main' into pkgwebtweaks

0ee520a

ditman approved these changes May 29, 2024

View reviewed changes

ditman added the autosubmit Merge PR when tree becomes green via auto submit App label May 29, 2024

auto-submit bot merged commit 910fabb into flutter:main May 29, 2024
74 checks passed

engine-flutter-autoroll mentioned this pull request May 30, 2024

Roll Packages from 31d332900d98 to 910fabb4886b (11 revisions) flutter/flutter#149321

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 30, 2024

910fabb Amend package:web tweaks to allow package:web roll (flutter/p…

1e26809

…ackages#6793)

reidbaker mentioned this pull request Jul 2, 2024

i151188 update espresso dependencies v2 reidbaker/packages#6

Closed

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Amend package:web tweaks to allow package:web roll #6793

Amend package:web tweaks to allow package:web roll #6793

srujzs commented May 24, 2024

flutter-dashboard bot commented May 24, 2024

srujzs commented May 24, 2024

ditman left a comment

srujzs commented May 28, 2024

srujzs commented May 29, 2024

ditman left a comment

Amend package:web tweaks to allow package:web roll #6793

Amend package:web tweaks to allow package:web roll #6793

Conversation

srujzs commented May 24, 2024

flutter-dashboard bot commented May 24, 2024

srujzs commented May 24, 2024

ditman left a comment

Choose a reason for hiding this comment

srujzs commented May 28, 2024

srujzs commented May 29, 2024

ditman left a comment

Choose a reason for hiding this comment