Skip to content

Conversation

@rajveermalviya
Copy link
Member

Also unpins and updates package:video_player along with the required changes for the new API.

Copy link
Collaborator

@chrisbobbe chrisbobbe left a comment

Choose a reason for hiding this comment

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

Thanks! Small comments below, and a quick manual smoke test on iOS was fine.

url_launcher: ^6.1.11
url_launcher_android: ">=6.1.0"
video_player: 2.9.5 # TODO unpin and upgrade to latest version
video_player: ^2.10.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does our Git history say about the reason for the pin; was it just the "small changes to the public API" mentioned and addressed here, or was there another reason? That would be helpful to make clear in the commit message.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the commit message.

url: "https://pub.dev"
source: hosted
version: "2.26.1"
version: "2.27.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm seeing some deprecation warnings from this upgrade:

simolus3/drift@4116b95

$ flutter analyze
Analyzing zulip-flutter...

   info • 'validateDropped' is deprecated and shouldn't be used. Use field in ValidationOptions instead • test/model/database_test.dart:204:28 • deprecated_member_use
   info • 'validateDropped' is deprecated and shouldn't be used. Use field in ValidationOptions instead • test/model/database_test.dart:214:59 • deprecated_member_use

2 issues found. (ran in 5.2s)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@rajveermalviya rajveermalviya added the maintainer review PR ready for review by Zulip maintainers label Jul 7, 2025
@rajveermalviya rajveermalviya requested a review from chrisbobbe July 7, 2025 19:25
@chrisbobbe
Copy link
Collaborator

Thanks! LGTM, marking for Greg's review.

@chrisbobbe chrisbobbe requested a review from gnprice July 7, 2025 19:29
@chrisbobbe chrisbobbe added the integration review Added by maintainers when PR may be ready for integration label Jul 7, 2025
@rajveermalviya
Copy link
Member Author

Thanks for the review @chrisbobbe! Pushed an update, PTAL.

@chrisbobbe chrisbobbe removed the maintainer review PR ready for review by Zulip maintainers label Jul 7, 2025
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

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

Thanks @rajveermalviya for taking care of this, and @chrisbobbe for the previous review!

All looks good with one nit. I'll fix that and merge.

url: "https://pub.dev"
source: hosted
version: "15.2.7"
version: "15.2.9"
Copy link
Member

Choose a reason for hiding this comment

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

Notable changes include bump to Firebase Android BoM (33.11.0 to 33.16.0)
and Firebase iOS SDK (11.13.0 to 11.15.0), changelog for those are at:
  https://firebase.google.com/support/release-notes/android
  https://firebase.google.com/support/release-notes/ios

In those bumps, one notable change on Android:
  https://firebase.google.com/support/release-notes/android#bom_v33-11-0
  https://github.com/firebase/firebase-android-sdk/pull/6980

That next-to-last link should say 16 instead of 11, right?

(I followed it and was puzzled that the Cloud Messaging entry there didn't seem to match the other link; then looked again at the version numbers.)

This commit is result of following commands:
  flutter pub upgrade --major-versions firebase_messaging firebase_core
  pod update --project-directory=ios/
  pod update --project-directory=macos/

Changelogs:
  https://pub.dev/packages/firebase_core/changelog#3151
  https://pub.dev/packages/firebase_messaging/changelog#1529

Notable changes include bump to Firebase Android BoM (33.11.0 to 33.16.0)
and Firebase iOS SDK (11.13.0 to 11.15.0), changelog for those are at:
  https://firebase.google.com/support/release-notes/android
  https://firebase.google.com/support/release-notes/ios

In those bumps, one notable change on Android:
  https://firebase.google.com/support/release-notes/android#bom_v33-16-0
  firebase/firebase-android-sdk#6980

And one on iOS:
  https://firebase.google.com/support/release-notes/ios#version_11140_-_june_3_2025
  firebase/firebase-ios-sdk#14846
Changelog:
  https://pub.dev/packages/drift/changelog#2270

One depcrecation:
  info • 'validateDropped' is deprecated and shouldn't be used. Use field in ValidationOptions instead • test/model/database_test.dart:204:28 • deprecated_member_use
  info • 'validateDropped' is deprecated and shouldn't be used. Use field in ValidationOptions instead • test/model/database_test.dart:214:59 • deprecated_member_use

because of:
  simolus3/drift@4116b95ef

So, update to the new API to fix analyzer warnings.
Changelog:
  https://pub.dev/packages/video_player/changelog#2100

One notable change is, support for opt-in use of
Flutter platform view:
  flutter/packages#8810

This commit also reverts the commit that pinned `video_player`
package:
  commit 2036178

    deps: Pin video_player to 2.9.5

    The latest version (2.10.0) makes significant changes internally
    and to the test API, which would require us to investigate and
    update our FakeVideoPlayerPlatform mock for tests.

    So, pin to the currently used version for now.

Upon investigating, the changes to the public API seem mostly
small, namely some deprecations in the platform interface class.
Which is only used in our tests, specifically by
FakeVideoPlayerPlatform. Otherwise, no behaviour change observed
while testing, as we are using the default options, i.e not using
Flutter platform view.
Changelog:
  https://docs.gradle.org/8.14.3/release-notes.html

The update includes bunch of bug fixes as this is a minor version
release.
@gnprice gnprice force-pushed the pr-upgrade-flutter branch from 8362239 to 2831798 Compare July 7, 2025 21:37
@gnprice gnprice merged commit 2831798 into zulip:main Jul 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration review Added by maintainers when PR may be ready for integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants