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.

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

fix(storage optimization): Removed and replaced used of PStore with G… #1730

Merged
merged 20 commits into from
Feb 21, 2025

Conversation

sienna-sterling
Copy link
Collaborator

…etStorage

Thank you so much for your contribution to FluffyChat ❤️❤️❤️

Please make sure that your Pull Request meet the following acceptance criteria:

  • Code formatting and import sorting has been done with dart format lib/ test/ and dart run import_sorter:main --no-comments
  • The commit message uses the format of Conventional Commits
  • The commit message describes what has been changed, why it has been changed and how it has been changed
  • Every new feature or change of the design/GUI is linked to an approved design proposal in an issue
  • Every new feature in the app or the build system has a strategy how this will be tested and maintained from now on for every release, e.g. a volunteer who takes over maintainership

Pull Request has been tested on:

  • Android
  • iOS
  • Browser (Chromium based)
  • Browser (Firefox based)
  • Browser (WebKit based)
  • Desktop Linux
  • Desktop Windows
  • Desktop macOS

@sienna-sterling sienna-sterling linked an issue Feb 7, 2025 that may be closed by this pull request
2 tasks
Copy link
Collaborator

@ggurdin ggurdin left a comment

Choose a reason for hiding this comment

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

@sienna-sterling Hello! This looks good. A few things:

  1. When we make changes to FluffyChat file (any file that's not under the /pangea folder) we wrap those changes in comments ("// #Pangea" and "// Pangea#") to indicate that it's our code and not FluffyChat's. Can you do this for the changes in chat_list.dart? Don't worry about changes that are already inside one of these blocks.
  2. All instances of GetStorage should be static

sienna-sterling and others added 3 commits February 7, 2025 17:20
…_login is the only one unfixed, I ran into additional errors I did not have time to fix today
@ggurdin
Copy link
Collaborator

ggurdin commented Feb 14, 2025

@sienna-sterling Hello! Can you also remove the changes related to link color? If you want to keep working on those, you can open a separate PR

@ggurdin
Copy link
Collaborator

ggurdin commented Feb 14, 2025

@sienna-sterling Each PLocalKey should correspond to one GetStorageBox. For instance, PLocalKey.showedUpdateDialog is only ever used in the GetStorage box with the key "version_storage" (see app_version_util.dart). There's a few instance here where a give PLocalKey is being written to one box and read from another. These are:

  1. PLocalKey.loginType is written to the box with the key "login_storage" (login.dart), but it's read from the box "settings_storage" (user_settings.dart). It's also being written to the box with the key "sso_storage" (sso_login_action.dart)
  2. PLocalKey.justInputtedCode is written to the box "chat_list_storage" (chat_list.dart) and is also written to the box "class_storage" (space_controller.dart), but is only ever read from "chat_list_storage" (chat_list_handle_space_tap.dart)
  3. PLocalKey.cachedClassCodeToJoin is written to the box "link_storage" (join_with_link.dart), but read and removed from the box "class_storage" (space_controller.dart)

These should all be one-to-one

@ggurdin
Copy link
Collaborator

ggurdin commented Feb 14, 2025

@sienna-sterling Looks good! Thank you.
Can you move the GetStorage box definitions into separate files? I think we only want to define them in files in the /pangea directory, inside subdirectories where they're being used. Maybe in files ending with _constants.dart (like analytics_constants.dart)

@ggurdin
Copy link
Collaborator

ggurdin commented Feb 14, 2025

@sienna-sterling Hello! The definitions for GetStorage boxes that were defined before this PR, like lemma_storage, don't need to be moved. Sorry if that wasn't clear.

@sienna-sterling
Copy link
Collaborator Author

@sienna-sterling Hello! The definitions for GetStorage boxes that were defined before this PR, like lemma_storage, don't need to be moved. Sorry if that wasn't clear.

@ggurdin How do I see which ones I have to put back?

@ggurdin
Copy link
Collaborator

ggurdin commented Feb 14, 2025

@sienna-sterling I think it's any of the GetStorage box names that exist in the current main branch. I would look at the file changes in gihub

@ggurdin
Copy link
Collaborator

ggurdin commented Feb 21, 2025

@sienna-sterling Thank you! Looks just about ready. I resolved some merge conflicts with the main branch, so make sure to pull those locally. I think the only thing left is to go through the changes files in github and remove any added spaces (in files that don't contain any storage box references) / commented out imports

@ggurdin ggurdin merged commit 4c1594d into main Feb 21, 2025
2 of 5 checks passed
@ggurdin ggurdin deleted the 1470-optimization-of-local-storage-use branch February 21, 2025 20:09
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.

Optimization of local storage use
2 participants