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

refactor: remove obsolete persistStoreData flag from PersistentStore #6300

Closed
wants to merge 1 commit into from

Conversation

dbjorge
Copy link
Contributor

@dbjorge dbjorge commented Dec 28, 2022

Details

When we originally introduced PersistentStore during the MV2->MV3 transition, it included a persistStoreData constructor flag which allowed us to gradually transition different stores to using persistence depending on whether we were working with an MV2 or MV3 build. This functionality is now obsolete - all PersistentStore-based stores always use this with a constant true input. This PR removes the obsolete flag.

This refactor is fairly simple but touches a lot of files, so I wanted to give it its own PR for ease of review.

Motivation

Clean up obsolete code

Context

Noticed while working on #6233

Pull request checklist

  • [n/a] Addresses an existing issue: #0000
  • Ran yarn fastpass
  • Added/updated relevant unit test(s) (and ran yarn test)
  • Verified code coverage for the changes made. Check coverage report at: <rootDir>/test-results/unit/coverage
  • PR title AND final merge commit title both start with a semantic tag (fix:, chore:, feat(feature-name):, refactor:). See CONTRIBUTING.md.
  • [n/a] (UI changes only) Added screenshots/GIFs to description above
  • [n/a] (UI changes only) Verified usability with NVDA/JAWS

@dbjorge dbjorge requested a review from a team as a code owner December 28, 2022 20:54
@dbjorge dbjorge closed this Dec 28, 2022
@dbjorge
Copy link
Contributor Author

dbjorge commented Dec 28, 2022

Ah, turns out this is still used for unified, oops

@codeofdusk
Copy link
Contributor

PR taken over at #6864.

codeofdusk added a commit that referenced this pull request Aug 4, 2023
… take 2 (#6864)

When we originally introduced `PersistentStore` during the MV2->MV3
transition, it included a `persistStoreData` constructor flag which
allowed us to gradually transition different stores to using persistence
depending on whether we were working with an MV2 or MV3 build. This
functionality is now obsolete - all `PersistentStore`-based stores
always use this with a constant `true` input. This commit removes the
obsolete flag.

This is a takeover of abandoned #6300, which was closed due to the
flag's usage in Unified. Now that Unified is gone, this change is
unblocked.

Noticed while working on #6233

Co-authored-by: Dan Bjorge <[email protected]>
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.

2 participants