Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[google_maps_flutter]: LatLng longitude loses precision in constructor #90574 #4374

Conversation

Murray-Meller
Copy link
Contributor

@Murray-Meller Murray-Meller commented Sep 23, 2021

This PR helps avoid unnecessary loss of longitude precision in the LatLng constructor.

See issue for more details.

Issues:

Fixes flutter/flutter#90574

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…s already within range to avoid unnecessary precision loss
…s already within range to avoid unnecessary precision loss
@google-cla
Copy link

google-cla bot commented Sep 23, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@Murray-Meller
Copy link
Contributor Author

@googlebot I signed it!

@google-cla
Copy link

google-cla bot commented Sep 23, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@MurrayMeller
Copy link

@googlebot I consent.

@google-cla google-cla bot added cla: yes and removed cla: no labels Sep 23, 2021
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

Two minor comment nits, but otherwise looks great. Thanks for all the added test coverage!

@Murray-Meller Murray-Meller force-pushed the 90574-Fix-LatLng-longitude-precision branch from 5921df3 to a69321e Compare September 23, 2021 21:38
@Murray-Meller
Copy link
Contributor Author

Murray-Meller commented Sep 23, 2021

Thanks for the review, @stuartmorgan. I have addressed both your comments.

I was wondering if you'd be able to help me understand more about my pipeline failures, where I could look and what I could do to get them fixed?

Incase it changes, these checks were failing:

  • ci.yaml validation
  • submit-queue

@ditman
Copy link
Member

ditman commented Sep 24, 2021

Incase it changes, these checks were failing:

ci.yaml validation
submit-queue

I can explain:

  • submit-queue tracks the status of tests of the commits in the master branch of flutter/plugins. If the tests don't pass in master, other PRs are blocked, until master recovers.
  • ci.yaml validation validates that the config for one of our continuous integration systems (LUCI) is OK. The case why it fails is because of a transient error, or because the branch was forked off at a point in master where the test was broken (and a fix was missing). (I checked this branch, and I didn't see any changes related to that validation, so it's probably a transient failure, and I restarted the check.)

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM

@stuartmorgan stuartmorgan requested a review from ditman October 1, 2021 15:32
@stuartmorgan
Copy link
Contributor

Adding @ditman for secondary review per Flutter policy.

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM!

@ditman ditman added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Oct 1, 2021
@fluttergithubbot fluttergithubbot merged commit d9490e7 into flutter:master Oct 1, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 1, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 1, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 1, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 2, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 2, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 2, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 2, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 2, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 5, 2021
NickalasB added a commit to NickalasB/plugins that referenced this pull request Oct 5, 2021
* master:
  [webview_flutter] Add zoomEnabled to webview flutter platform interface (flutter#4404)
  [ci] Remove obsolete Dockerfile (flutter#4405)
  Fix order-dependant platform interface tests (flutter#4406)
  [google_maps_flutter]: LatLng longitude loses precision in constructor #90574 (flutter#4374)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 5, 2021
@MurrayMeller
Copy link

Thanks @stuartmorgan & @ditman.

One last question if I may, what is the timeline/process for this making it into a plugin release?

@ditman
Copy link
Member

ditman commented Oct 6, 2021

One last question if I may, what is the timeline/process for this making it into a plugin release?

@MurrayMeller For each push to master, a set of post-submit + release actions are generated. See them here for this PR:

When all the "Cirrus CI" tests pass, the "Release" action gets triggered, and if that's successful, it's already available to people:

(A flutter pub upgrade should grab the latest available version of the platform_interface package)

@MurrayMeller
Copy link

Thank again, @ditman. Much appreciated.

@ditman
Copy link
Member

ditman commented Oct 6, 2021

@MurrayMeller thanks again for your contribution!! Keep them coming!

mgonzalezc pushed a commit to mgonzalezc/plugins that referenced this pull request Oct 12, 2021
* master: (1126 commits)
  [webview_flutter] Adjust test URLs again (flutter#4407)
  [google_sign_in] Add serverAuthCode attribute to google_sign_in_platform_interface user data (flutter#4179)
  [camera] Add filter for unsupported cameras on Android (flutter#4418)
  [webview_flutter] Update webview platform interface with new methods for running JavaScript. (flutter#4401)
  [webview_flutter] Add zoomEnabled to webview flutter platform interface (flutter#4404)
  [ci] Remove obsolete Dockerfile (flutter#4405)
  Fix order-dependant platform interface tests (flutter#4406)
  [google_maps_flutter]: LatLng longitude loses precision in constructor #90574 (flutter#4374)
  [google_maps_flutter] Add Marker drag events (flutter#2838)
  [flutter_plugin_tools] Validate pubspec description (flutter#4396)
  Add file_selector to the repo list (flutter#4395)
  [in_app_purchase] Fix in_app_purchase_android/README.md (flutter#4363)
  [google_maps_flutter_web] Add Marker drag events (flutter#4385)
  [webview_flutter] Fixed todos in FlutterWebView.java (flutter#4394)
  Handle `PurchaseStatus.restored` correctly in example. (flutter#4393)
  Handle restored purchases in iOS example app (flutter#4392)
  [file_selector] Remove custom analysis options (flutter#4382)
  [flutter_plugin_tools] Check licenses in Kotlin (flutter#4373)
  Fixed _CastError when running example App (flutter#4390)
  [in_app_purchase] Ensure the introductoryPriceMicros field is transported as a String. (flutter#4370)
  ...

# Conflicts:
#	packages/quick_actions/ios/Classes/FLTQuickActionsPlugin.m
camsim99 pushed a commit to camsim99/plugins that referenced this pull request Oct 18, 2021
amantoux pushed a commit to amantoux/plugins that referenced this pull request Dec 11, 2021
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes p: google_maps_flutter waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Google Maps Flutter: LatLng longitude loses precision in constructor
5 participants