Skip to content

Switch to using maybeLocaleOf instead of localeOf - #443

Merged
dnfield merged 1 commit into
dnfield:masterfrom
gspencergoog:locale_of
Dec 4, 2020
Merged

dnfield merged 1 commit into
dnfield:masterfrom
gspencergoog:locale_of

Conversation

@gspencergoog

Copy link
Copy Markdown
Contributor

See discussion at flutter/flutter#68911

Obv, 1.24.0-8.0.pre hasn't been released yet, but will include the appropriate change when it has been.

@gspencergoog

Copy link
Copy Markdown
Contributor Author

cc @dnfield

@dnfield dnfield left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM

@dnfield dnfield left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM

@dnfield

dnfield commented Nov 6, 2020

Copy link
Copy Markdown
Owner

Hm. Looks like dart changed some rules. I can push a fix up for it and then merge it into this.

@dnfield

dnfield commented Nov 12, 2020

Copy link
Copy Markdown
Owner

Now coveralls is failing? :\

@gspencergoog

Copy link
Copy Markdown
Contributor Author

**The specified coverage format is not supported.** Yeah, that's weird.

@gspencergoog

Copy link
Copy Markdown
Contributor Author

I just rebased and pushed. This isn't expected to pass though, until the next dev release, since the requested SDK version doesn't exist yet.

@gspencergoog

Copy link
Copy Markdown
Contributor Author

I can switch this to a draft PR until then, if you want.

@dnfield

dnfield commented Nov 12, 2020

Copy link
Copy Markdown
Owner

Yeah let's do that, as long as it doesn't block progress upstream.

@gspencergoog
gspencergoog marked this pull request as draft November 12, 2020 19:41
@gspencergoog
gspencergoog marked this pull request as ready for review November 20, 2020 23:18
@gspencergoog
gspencergoog force-pushed the locale_of branch 2 times, most recently from 5762a0f to 421514e Compare November 20, 2020 23:22
@gspencergoog
gspencergoog marked this pull request as draft November 20, 2020 23:23
@gspencergoog

Copy link
Copy Markdown
Contributor Author

Hmm. I tried re-running this now that 1.24.0-10.2.pre is out in dev (and beta), and it fails on Stable. Is that expected? It also still fails coveralls, but that seems like a CI problem.

@gspencergoog
gspencergoog marked this pull request as ready for review December 4, 2020 19:31
@gspencergoog

Copy link
Copy Markdown
Contributor Author

OK, @dnfield trying again now that the pubspec has advanced the Flutter version.

@gspencergoog

Copy link
Copy Markdown
Contributor Author

OK, so maybe I just need to make this change on the nullsafety branch? Seems like the master branch for flutter_svg must be failing for all PRs right now, since this change doesn't modify the pubspec.yaml anymore.

@dnfield

dnfield commented Dec 4, 2020

Copy link
Copy Markdown
Owner

Coveralls is sometimes failing for reasons I don't understand, this change LGTM and should be fine to land as is.

@dnfield
dnfield merged commit 016a85e into dnfield:master Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants