Skip to content
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

chore(firebase_auth_web, firebase_ui_localizations): Upgrades intl to 0.18.0 #10317

Closed
wants to merge 6 commits into from

Conversation

Rexios80
Copy link
Contributor

Description

Upgrades intl to version 0.18.0 in all packages

Related Issues

N/A

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

@Rexios80 Rexios80 requested a review from lesnitsky as a code owner January 24, 2023 20:08
@Rexios80 Rexios80 changed the title chore(firebase_auth_web, firebase_ui_localizations) Upgrades intl to 0.18.0 chore(firebase_auth_web, firebase_ui_localizations): Upgrades intl to 0.18.0 Jan 24, 2023
@russellwheatley
Copy link
Member

@Rexios80 You'll have to update/fix the failing tests before review. Thanks

@Rexios80
Copy link
Contributor Author

Pretty sure things are failing because of flutter 3.7 deprecations which is outside the scope of this PR to fix unless you want it done here

@russellwheatley
Copy link
Member

We've attempted this before. If the SDK relies on version 0.17, then we aren't able to proceed as far as I can tell.

@Rexios80
Copy link
Contributor Author

Rexios80 commented Feb 6, 2023

This should be possible when this issue closes: flutter/flutter#117163

@zeshuaro
Copy link

Currently there seems to be 3 opened PRs attempting to bump the version of intl, maybe we should focus on one of them and mark the others as duplicated?

Anyway, I just want to propose to relax the version of intl to >=0.17.0 <1.0.0 given that there aren't breaking changes from 0.17.0 to 0.18.0 and to assume that no breaking changes would be released in minor version updates.

When intl releases 0.19.0, the exact same issue will hit again and the proposed range version should get around with it. Also see this related issue on Pub about version resolving for 0.x.x versions.

If we don't feel safe for minor releases of intl, can we at least set the version to >=0.17.0 <0.19.0 so that the Firebase packages will work for both 0.17.0 and 0.18.0?

I currently have a project that also uses intl and I'm having version resolve issues with a lot of the other packages that also use intl. So I would appreciate if a ranged version is used here so that I can gradually transition with the dependencies upgrade on my project while waiting for all the packages to also support the latest version of intl.

@Rexios80
Copy link
Contributor Author

Minor version changes on version 0.x.x are treated as breaking changes. We cannot assume that version 0.19.0 won't have breaking changes, because that's not how the versioning system is designed. What probably needs to happen is that intl needs a 1.0.0 release so that minor version changes aren't treated as breaking, but that is well outside the scope of this repo.

We could have a constraint that includes both 0.17.0 and 0.18.0, but that's probably not worth it since the issue right now is not supporting 0.18.0. This is just a growing pain with 0.18.0 that when resolved won't matter anymore.

If you're having issues with the intl version in your projects, just add it to the dependency_overrides for now:

dependency_overrides:
  intl: ^0.18.0

If there were actually no breaking changes, this will solve your dependency resolution issues.

@Rexios80
Copy link
Contributor Author

dart-lang/i18n#458

@Rexios80
Copy link
Contributor Author

Rexios80 commented Feb 27, 2023

Is that CI failure my fault?

@Rexios80
Copy link
Contributor Author

Seems we are still waiting on a flutter release with flutter_localizations that uses intl: 0.18.0

@Rexios80
Copy link
Contributor Author

Now CI is failing because of Flutter 3.10 changes. Waiting on #10944

@Rexios80
Copy link
Contributor Author

This should have been dealt with in #10944

@Rexios80 Rexios80 closed this May 11, 2023
@firebase firebase locked and limited conversation to collaborators Jun 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants