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.

Sign up for GitHub

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

Jump to bottom

[shared_preferences] Tool for migrating from legacy shared_preferences to shared_preferences_async #8229

Merged
merged 8 commits into from
Jan 24, 2025
Merged

[shared_preferences] Tool for migrating from legacy shared_preferences to shared_preferences_async #8229

merged 8 commits into from
Jan 24, 2025
+334 −15

Conversation

tarrinneal
Copy link
Contributor

@tarrinneal tarrinneal commented Dec 4, 2024 •
edited
Loading

@tarrinneal tarrinneal marked this pull request as ready for review December 13, 2024 22:28
@tarrinneal
Copy link
Contributor Author

tarrinneal commented Dec 14, 2024 •
edited
Loading

@stuartmorgan this is ready for review now

stuartmorgan
stuartmorgan requested changes Jan 3, 2025
View reviewed changes
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.

The README should be updated to explain this, in place of the discussion about manual migration.

packages/shared_preferences/shared_preferences/lib/tools/legacy_to_async_migration_tool.dart Outdated Show resolved Hide resolved
packages/shared_preferences/shared_preferences/CHANGELOG.md Outdated Show resolved Hide resolved
packages/shared_preferences/shared_preferences/CHANGELOG.md Outdated Show resolved Hide resolved
...nces/shared_preferences/example/integration_test/shared_preferences_migration_tool_test.dart Outdated Show resolved Hide resolved
...nces/shared_preferences/example/integration_test/shared_preferences_migration_tool_test.dart Outdated Show resolved Hide resolved
packages/shared_preferences/shared_preferences/lib/tools/legacy_to_async_migration_tool.dart Outdated Show resolved Hide resolved
packages/shared_preferences/shared_preferences/lib/tools/legacy_to_async_migration_tool.dart Outdated
SharedPreferences legacySharedPreferencesInstance,
SharedPreferencesOptions sharedPreferencesAsyncOptions,
String migrationCompletedKey, {
bool clearLegacyPreferences = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

This appears to not do anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

forgot about that. Is this a feature that's worth adding? I hadn't yet as it seemed like it could be problematic.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could wait and see if anyone asks. On one hand, I would generally expect a migration to move data instead of duplicating it. On the other hand, it shouldn't be a lot of data.

My one real hesitation here is that if someone isn't using an allow list, on most platforms this would mean that an unprefixed new-style prefs object would be loading all the old prefs too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also means that items not set by our plugin would get deleted. Potentially if not used carefully (or written carefully) it could write to the same file, then delete the prefs it just wrote.

Copy link
Contributor

Choose a reason for hiding this comment

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

It also means that items not set by our plugin would get deleted.

The common case will be that people are not setting a prefix (since that was added pretty recently in the shared_preferences lifetime, and is opt-in even once available). In that scenario, clearing legacy prefs can't delete anything we didn't write.

packages/shared_preferences/shared_preferences/lib/tools/legacy_to_async_migration_tool.dart Outdated Show resolved Hide resolved
packages/shared_preferences/shared_preferences/lib/tools/legacy_to_async_migration_tool.dart Outdated Show resolved Hide resolved
packages/shared_preferences/shared_preferences/lib/tools/legacy_to_async_migration_tool.dart Outdated Show resolved Hide resolved
stuartmorgan
stuartmorgan requested changes Jan 8, 2025
View reviewed changes
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.

<seinfeldmeme>You thumbs-up'd my comment about updating the README, but you didn't update the README</seinfeldmeme> 🙂

packages/shared_preferences/shared_preferences/lib/util/legacy_to_async_migration_util.dart Outdated Show resolved Hide resolved
packages/shared_preferences/shared_preferences/lib/util/legacy_to_async_migration_util.dart Outdated Show resolved Hide resolved
@tarrinneal
Copy link
Contributor Author

<seinfeldmeme>You thumbs-up'd my comment about updating the README, but you didn't update the README</seinfeldmeme> 🙂

I use the resolve button to track work I have or haven't done yet, since that comment isn't resolvable, I missed it.

@tarrinneal
Copy link
Contributor Author

<seinfeldmeme>You thumbs-up'd my comment about updating the README, but you didn't update the README</seinfeldmeme> 🙂

RESOLVED

stuartmorgan
stuartmorgan approved these changes Jan 23, 2025
View reviewed changes
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 once you re-run snippet generation.

@tarrinneal
Copy link
Contributor Author

LGTM once you re-run snippet generation.

I'm waiting for the revert pr to land before I handle conflicts and merge this pr.

@tarrinneal tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 24, 2025
@auto-submit auto-submit bot merged commit 3d28a90 into flutter:main Jan 24, 2025
77 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 24, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 24, 2025
@engine-flutter-autoroll
3d28a9097 [shared_preferences] Tool for migrating from legacy shared_…
fa88022 
…preferences to shared_preferences_async (flutter/packages#8229)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 24, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 24, 2025
@engine-flutter-autoroll
3d28a9097 [shared_preferences] Tool for migrating from legacy shared_…
76e9d19 
…preferences to shared_preferences_async (flutter/packages#8229)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 24, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 24, 2025
@engine-flutter-autoroll
3d28a9097 [shared_preferences] Tool for migrating from legacy shared_…
e3538a7 
…preferences to shared_preferences_async (flutter/packages#8229)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 27, 2025
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 27, 2025
@engine-flutter-autoroll
3d28a9097 [shared_preferences] Tool for migrating from legacy shared_…
c8cb072 
…preferences to shared_preferences_async (flutter/packages#8229)
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Jan 27, 2025
@engine-flutter-autoroll
Roll Packages from 3d3ab7bcb07e to 258f6dce6a49 (19 revisions) (#162254)
a2c56f3 
flutter/packages@3d3ab7b...258f6dc

2025-01-24 adsonpleal@gmail.com [shared_preferences] Add shared
preferences devtool (flutter/packages#8494)
2025-01-24 tarrinneal@gmail.com [shared_preferences] update List<String>
encode/decode (flutter/packages#8335)
2025-01-24 engine-flutter-autoroll@skia.org Manual roll Flutter from
c1561a4 to c1ffaa9 (21 revisions) (flutter/packages#8498)
2025-01-24 stuartmorgan@google.com [ios_platform_images] Switch to
`loadImage` (flutter/packages#8216)
2025-01-24 mchudy@users.noreply.github.com [camera] Remove OCMock from
CameraExposureTests and CameraFocusTests (flutter/packages#8351)
2025-01-24 tarrinneal@gmail.com [shared_preferences] Tool for migrating
from legacy shared_preferences to shared_preferences_async
(flutter/packages#8229)
2025-01-23 stuartmorgan@google.com Revert "[shared_preferences] Add
shared preferences devtool" (flutter/packages#8493)
2025-01-23 20254485+kaboc@users.noreply.github.com [go_router] Fix
return type of current state getter to be non-nullable
(flutter/packages#8173)
2025-01-23 engine-flutter-autoroll@skia.org Manual roll Flutter from
b2f515f to c1561a4 (18 revisions) (flutter/packages#8491)
2025-01-23 tarrinneal@gmail.com [pigeon] fixes event channel dart
instance name usage and adds test (flutter/packages#8483)
2025-01-23 51104750+AffanShaikhsurab@users.noreply.github.com [go
_route] fragment parameter added (flutter/packages#8232)
2025-01-23 mchudy@users.noreply.github.com [in_app_purchase] Update
in_app_purchase_android version in in_app_purchase
(flutter/packages#8463)
2025-01-23 stuartmorgan@google.com [image_picker] Reference alternate
macOS implementations (flutter/packages#8487)
2025-01-23 32538273+ValentinVignal@users.noreply.github.com [rfw]
Activate leak testing (flutter/packages#8370)
2025-01-23 32538273+ValentinVignal@users.noreply.github.com
[video_player] Activate leak testing (flutter/packages#8379)
2025-01-23 engine-flutter-autoroll@skia.org Manual roll Flutter from
b9e86a5 to b2f515f (42 revisions) (flutter/packages#8482)
2025-01-23 olli.helenius@codemate.com [camera] Add API support query for
image streaming (app-facing) (flutter/packages#8422)
2025-01-23 engine-flutter-autoroll@skia.org Manual roll Flutter from
b9e86a5 to eb6af3d (13 revisions) (flutter/packages#8473)
2025-01-23 adsonpleal@gmail.com [shared_preferences] Add shared
preferences devtool (flutter/packages#8322)

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 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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartmorgan stuartmorgan stuartmorgan approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: shared_preferences
Projects
None yet
Milestone
No milestone
2 participants
@tarrinneal @stuartmorgan