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

[google_sign_in] Updates google_sign_in_platform_interfaces adding parametrized clientId #3686

Merged
merged 7 commits into from
Mar 6, 2021
Merged

[google_sign_in] Updates google_sign_in_platform_interfaces adding parametrized clientId #3686

merged 7 commits into from
Mar 6, 2021

Conversation

ggirotto
Copy link
Contributor

@ggirotto ggirotto commented Mar 5, 2021

This PR adds support to #3640 by parametrizing clientId property in MethodChannelGoogleSignIn

List which issues are fixed by this PR. You must list at least one issue.

Same issues as #3640

Pre-launch Checklist

  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • 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 Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • 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 signed the CLA.
  • All existing and new tests are passing.

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

@google-cla google-cla bot added the cla: yes label Mar 5, 2021
@ggirotto
Copy link
Contributor Author

ggirotto commented Mar 5, 2021

Testes to google_sign_in were added at #3640, which will complement the current PR changes

@ggirotto ggirotto mentioned this pull request Mar 5, 2021
Merged
11 tasks
@cyanglaz
Copy link
Contributor

cyanglaz commented Mar 5, 2021

platform interface change should also be testable through unit tests.

@ggirotto
Copy link
Contributor Author

ggirotto commented Mar 5, 2021

@cyanglaz Updated Other functions pass through arguments to the channel test which covers parametrized arguments in init function

cyanglaz
cyanglaz approved these changes Mar 5, 2021
View reviewed changes
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

cyanglaz
cyanglaz suggested changes Mar 5, 2021
View reviewed changes
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

Need to update the minor version because there's a public api change.

@ggirotto
Copy link
Contributor Author

ggirotto commented Mar 5, 2021

@cyanglaz done!

cyanglaz
cyanglaz reviewed Mar 6, 2021
View reviewed changes
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

Actually im sorry. The clientId field was an existing unused filed in the public API, so your initial version was correct
ill change it back

cyanglaz
cyanglaz reviewed Mar 6, 2021
View reviewed changes
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

Rebasing master should fix ci

@ggirotto
Copy link
Contributor Author

ggirotto commented Mar 6, 2021

@cyanglaz Now I think its good. Please let me know if it need more adjustments

cyanglaz
cyanglaz approved these changes Mar 6, 2021
View reviewed changes
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM

@cyanglaz cyanglaz merged commit e615682 into flutter:master Mar 6, 2021
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 6, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 6, 2021
@engine-flutter-autoroll
e615682 [google_sign_in] Updates google_sign_in_platform_interfaces a…
3c4c7c5 
…dding parametrized `clientId` (flutter/plugins#3686)
fluttergithubbot pushed a commit to flutter/flutter that referenced this pull request Mar 8, 2021
@engine-flutter-autoroll
e615682 [google_sign_in] Updates google_sign_in_platform_interfaces a…
8d4aac1 
…dding parametrized `clientId` (flutter/plugins#3686) (#77479)
kf6gpe added a commit to flutter/flutter that referenced this pull request Mar 8, 2021
@kf6gpe
Revert "e6156827a [google_sign_in] Updates google_sign_in_platform_in…
2baba2f 
…terfaces adding parametrized `clientId` (flutter/plugins#3686) (#77479)"

This reverts commit 8d4aac1.
@cyanglaz cyanglaz mentioned this pull request Mar 8, 2021
Merged
11 tasks
NickalasB added a commit to NickalasB/plugins that referenced this pull request Mar 8, 2021
@NickalasB
Merge branch 'master' into add_zoom_to_android_webview
fc41c1e 
* master:
  [google_sign_in] fix test(flutter#3690)
  [extension_google_sign_in_as_googleapis_auth] Update import (flutter#3689)
  [google_sign_in] Updates google_sign_in_platform_interfaces adding parametrized `clientId` (flutter#3686)
  Import flutter_test for future compatibility (flutter#3665)
  [ci] Disable analyze on stable for web plugins that contains null safety integration tests.  (flutter#3681)
  Bring HTML inputs into view automatically (flutter#3655)
  [in_app_purchase] presentCodeRedemptionSheet (flutter#3274)
  [google_maps_flutter_web] Downgrade mockito in example app. (flutter#3679)
  Update CI config for Flutter 2 (flutter#3674)
  [image_picker] fix flutter/flutter#71927 (flutter#3676)
  [google_maps_flutter_web] Move integration tests to example. (flutter#3675)
  [google_maps_flutter_web] Make google_maps_flutter_web work with latest plugins  (flutter#3673)

# Conflicts:
#	packages/webview_flutter/CHANGELOG.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cyanglaz cyanglaz cyanglaz approved these changes

Assignees
No one assigned
Labels
cla: yes p: google_sign_in
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ggirotto @cyanglaz