[shared_preferences] Ports SharedPreferencesAndroid to Pigeon #3321

AmanNegi · 2023-02-28T16:13:41Z

This PR ports the SharedPreferences-Android to Pigeon. It was earlier created in flutter/plugins however it was ported to flutter/packages.

fixes flutter/flutter#95013
fixes flutter/flutter#117914

Previous PR: flutter/plugins#7015

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. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
[✅] I signed the CLA.
[✅] The title of the PR starts with the name of the package 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, or this PR is exempt from version changes.
[✅] I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
[✅] I updated/added relevant documentation (doc comments with ///).
[✅] I added new tests to check the change I am making, or this 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.

tarrinneal · 2023-03-01T02:56:46Z

Have all of the requested changes from the closed pr been made?

AmanNegi · 2023-03-01T03:07:44Z

Hey there @tarrinneal, I was requested by @stuartmorgan for a few changes however they were lost in the transition of moving from /plugins to /packages. I am working on those and will update you asap.

stuartmorgan · 2023-03-01T03:09:36Z

[✅] The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]

Please don't check boxes without having done the checklist item; it defeats the point of the checklist.

AmanNegi · 2023-03-01T04:00:13Z

[✅] The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]

Please don't check boxes without having done the checklist item; it defeats the point of the checklist.

Sorry for that got carried in the switching transition, fixed it now. I did not receive any response regarding this. Let me know if it seems alright?

stuartmorgan · 2023-03-02T15:01:47Z

I did not receive any response regarding this. Let me know if it seems alright?

You have not addressed my original comment, so no, this still has the same problem. This PR needs actual tests.

stuartmorgan

(Updating review status based on the above.)

stuartmorgan · 2023-03-21T19:54:02Z

@AmanNegi Are you still planning on updating this PR with tests?

AmanNegi · 2023-03-22T02:46:51Z

Yeah, but I'm unable to think as to how to test theSharedPreferences object and its functionalities. Only way is to mock all the functions.

I will provide you the updated tests, let me know if they are fine

stuartmorgan · 2023-03-22T10:27:27Z

I'm unable to think as to how to test theSharedPreferences object

As explained previously, the goal is not to test SharedPreferences, it is to test the plugin's code, which is SharedPreferencesPlugin and MethodCallHandlerImpl.

…efs-android-to-pigeon

tarrinneal · 2023-04-10T20:59:38Z

adding @bparrishMines as a reviewer, since I can't review my own code in this pr

packages/shared_preferences/shared_preferences_android/CHANGELOG.md

packages/shared_preferences/shared_preferences_android/pigeons/messages.dart

...ages/shared_preferences/shared_preferences_android/test/shared_preferences_android_test.dart

...ndroid/android/src/test/java/io/flutter/plugins/sharedpreferences/SharedPreferencesTest.java

packages/shared_preferences/shared_preferences_android/lib/shared_preferences_android.dart

...roid/android/src/main/java/io/flutter/plugins/sharedpreferences/SharedPreferencesPlugin.java

stuartmorgan · 2023-04-12T03:23:29Z

...roid/android/src/main/java/io/flutter/plugins/sharedpreferences/SharedPreferencesPlugin.java

+ private void setUp(@NonNull BinaryMessenger messenger, @NonNull Context context) {
+ preferences = context.getSharedPreferences(SHARED_PREFERENCES_NAME, Context.MODE_PRIVATE);
+ try {
+ SharedPreferencesApi.setup(messenger, this);


When detaching from the engine this should be replaced with null to remove the handler.

...roid/android/src/main/java/io/flutter/plugins/sharedpreferences/SharedPreferencesPlugin.java

stuartmorgan

LGTM with some final nits for things I missed. Thanks for getting this over the finish line!

...roid/android/src/main/java/io/flutter/plugins/sharedpreferences/SharedPreferencesPlugin.java

packages/shared_preferences/shared_preferences_android/lib/shared_preferences_android.dart

stuartmorgan · 2023-04-13T19:16:27Z

Tree status is stale here; overriding.

AmanNegi · 2023-04-13T19:24:17Z

Thanks a lot @tarrinneal for helping with this. Also special thanks for @stuartmorgan for maintaining the project and guiding us.

tarrinneal · 2023-04-13T22:08:25Z

Thanks a lot @tarrinneal for helping with this. Also special thanks for @stuartmorgan for maintaining the project and guiding us.

Thank you for getting this going!

…flutter/packages#3321)

flutter/packages@d01f4ea...e4ec155 2023-04-14 stuartmorgan@google.com [shared_preferences] Remove Android lint baseline (flutter/packages#3707) 2023-04-13 37607224+AmanNegi@users.noreply.github.com [shared_preferences] Ports SharedPreferencesAndroid to Pigeon (flutter/packages#3321) 2023-04-13 10687576+bparrishMines@users.noreply.github.com [webview_flutter_android] Fix Android lint warnings (flutter/packages#3675) 2023-04-13 gautier.bayzelon@gmail.com [go_router_builder] Use only one .where() (flutter/packages#3698) 2023-04-13 engine-flutter-autoroll@skia.org Roll Flutter from 3b4b149 to be45eb2 (21 revisions) (flutter/packages#3701) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…r#3321) [shared_preferences] Ports SharedPreferencesAndroid to Pigeon

Recreating PR from flutter/plugins

f8260d7

AmanNegi requested a review from reidbaker as a code owner February 28, 2023 16:13

github-actions bot added p: shared_preferences platform-android labels Feb 28, 2023

reidbaker requested a review from tarrinneal February 28, 2023 16:33

tarrinneal requested a review from stuartmorgan March 1, 2023 02:56

AmanNegi changed the title ~~feat(Package): Ports SharedPreferencesAndroid to Pigeon~~ [shared_preferences] Ports SharedPreferencesAndroid to Pigeon Mar 1, 2023

stuartmorgan requested changes Mar 2, 2023

View reviewed changes

tarrinneal added 2 commits April 5, 2023 14:59

Merge branch 'main' of github.com:flutter/packages into port-sharedpr…

312dce7

…efs-android-to-pigeon

update to latest api

211b2cf

tarrinneal force-pushed the port-sharedprefs-android-to-pigeon branch from 752b9c5 to a87ce97 Compare April 6, 2023 02:07

tarrinneal requested review from ditman, bparrishMines and chunhtai as code owners April 6, 2023 02:07

github-actions bot added p: go_router p: google_sign_in p: pigeon p: webview_flutter platform-ios platform-macos platform-web platform-windows labels Apr 6, 2023

tarrinneal added 3 commits April 10, 2023 11:45

import nonnull

b7dfefa

Every thing in the world is now nonnull

f11df81

format

50e4b8c

tarrinneal requested a review from bparrishMines April 10, 2023 20:59

stuartmorgan requested changes Apr 11, 2023

View reviewed changes

reidbaker approved these changes Apr 11, 2023

View reviewed changes

tarrinneal added 3 commits April 11, 2023 13:47

most nits (still need to merge files)

56518bf

merge files

279ec4a

comments and tests

4872c06

tarrinneal requested a review from stuartmorgan April 11, 2023 21:48

format pigeons (it's good for their health)

5b34294

tarrinneal removed their request for review April 12, 2023 01:05

stuartmorgan requested changes Apr 12, 2023

View reviewed changes

nits

deb7e49

tarrinneal requested a review from stuartmorgan April 12, 2023 20:16

stuartmorgan approved these changes Apr 13, 2023

View reviewed changes

final nits

403826c

tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 13, 2023

stuartmorgan added the warning: land on red to fix tree breakage Override tree-status signal (land even with closed tree), combine with the autosubmit label. label Apr 13, 2023

auto-submit bot merged commit 9852b41 into flutter:main Apr 13, 2023

engine-flutter-autoroll mentioned this pull request Apr 14, 2023

Roll Packages from d01f4eaabb54 to e4ec15590935 (5 revisions) flutter/flutter#124863

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 14, 2023

9852b41 [shared_preferences] Ports SharedPreferencesAndroid to Pigeon (…

c101f82

…flutter/packages#3321)

reidbaker mentioned this pull request Apr 25, 2023

[various] update agp and gradle for all examples in packages #3822

Merged

11 tasks

nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023

[shared_preferences] Ports SharedPreferencesAndroid to Pigeon (flutte…

db74b53

…r#3321) [shared_preferences] Ports SharedPreferencesAndroid to Pigeon

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[shared_preferences] Ports SharedPreferencesAndroid to Pigeon #3321

[shared_preferences] Ports SharedPreferencesAndroid to Pigeon #3321

AmanNegi commented Feb 28, 2023 •

edited by tarrinneal

Loading

tarrinneal commented Mar 1, 2023

AmanNegi commented Mar 1, 2023

stuartmorgan commented Mar 1, 2023

AmanNegi commented Mar 1, 2023

stuartmorgan commented Mar 2, 2023

stuartmorgan left a comment

stuartmorgan commented Mar 21, 2023

AmanNegi commented Mar 22, 2023 •

edited

Loading

stuartmorgan commented Mar 22, 2023

tarrinneal commented Apr 10, 2023

stuartmorgan Apr 12, 2023

stuartmorgan left a comment

stuartmorgan commented Apr 13, 2023

AmanNegi commented Apr 13, 2023

tarrinneal commented Apr 13, 2023 •

edited

Loading

[shared_preferences] Ports SharedPreferencesAndroid to Pigeon #3321

[shared_preferences] Ports SharedPreferencesAndroid to Pigeon #3321

Conversation

AmanNegi commented Feb 28, 2023 • edited by tarrinneal Loading

Pre-launch Checklist

tarrinneal commented Mar 1, 2023

AmanNegi commented Mar 1, 2023

stuartmorgan commented Mar 1, 2023

AmanNegi commented Mar 1, 2023

stuartmorgan commented Mar 2, 2023

stuartmorgan left a comment

Choose a reason for hiding this comment

stuartmorgan commented Mar 21, 2023

AmanNegi commented Mar 22, 2023 • edited Loading

stuartmorgan commented Mar 22, 2023

tarrinneal commented Apr 10, 2023

stuartmorgan Apr 12, 2023

Choose a reason for hiding this comment

stuartmorgan left a comment

Choose a reason for hiding this comment

stuartmorgan commented Apr 13, 2023

AmanNegi commented Apr 13, 2023

tarrinneal commented Apr 13, 2023 • edited Loading

AmanNegi commented Feb 28, 2023 •

edited by tarrinneal

Loading

AmanNegi commented Mar 22, 2023 •

edited

Loading

tarrinneal commented Apr 13, 2023 •

edited

Loading