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

refactor(bg/storage): use separate key to toggle continuous payments #727

Merged
merged 4 commits into from
Nov 28, 2024
Merged

refactor(bg/storage): use separate key to toggle continuous payments #727

merged 4 commits into from
Nov 28, 2024

Conversation

sidvishnoi
Copy link
Member

@sidvishnoi sidvishnoi commented Nov 25, 2024 •
edited
Loading

Context

Will help with #623 as decided in #654 (comment)

Then in future enabled will mean entire extension enabled or not, not just continuous payments.

Changes proposed in this pull request

  • Rename Storage.enabled to Storage.continuousPaymentsEnabled
  • Migrate extension storage from version 3 to 4
    • set continuousPaymentsEnabled to previous value of enabled
    • set enabled to true for #623
  • Update entire codebase to reflect this rename

Notes to reviewer

  • A shorter name suggestion is definitely welcome.

@github-actions github-actions bot added area: background Improvements or additions to extension background script area: popup Improvements or additions to extension popup labels Nov 25, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 25, 2024 •
edited
Loading

Extension builds preview

Name Link
Latest commit 3abdf57
Latest job logs Run #12067669602
BadgeDownload
BadgeDownload

raducristianpopa
raducristianpopa reviewed Nov 27, 2024
View reviewed changes
Copy link
Member

@raducristianpopa raducristianpopa left a comment

Choose a reason for hiding this comment

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

Now thinking about it, we might not even need to delete the enabled key now, if we plan to introduce the global enabled flag. We can just keep it for when that lands.

src/background/services/storage.ts Outdated Show resolved Hide resolved
@sidvishnoi
Copy link
Member Author

Yes, but its value isn't same as current meaning of enabled. We'll add it back (as new migration) with its new meaning.

@sidvishnoi
Copy link
Member Author

Update: oh yeah we can keep it. Just not use its value or set it yet.

@DarianM
Copy link
Member

DarianM commented Nov 28, 2024

LGTM

@sidvishnoi sidvishnoi changed the title refactor(bg/storage): rename enabled to continuousPaymentsEnabled refactor(bg/storage): use separate key to toggle continuous payments Nov 28, 2024
@sidvishnoi sidvishnoi merged commit 27d260c into main Nov 28, 2024
9 checks passed
@sidvishnoi sidvishnoi deleted the rename-storage/enabled branch November 28, 2024 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raducristianpopa raducristianpopa raducristianpopa approved these changes

@DarianM DarianM Awaiting requested review from DarianM

Assignees
No one assigned
Labels
area: background Improvements or additions to extension background script area: popup Improvements or additions to extension popup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sidvishnoi @DarianM @raducristianpopa