-
Notifications
You must be signed in to change notification settings - Fork 895
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
Use base::SequenceBound to hold an instance of LedgerDatabase #9203
Conversation
9d3da8e
to
a4a11cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in previous PR, would like to make sure everything works at runtime for iOS before this is merged
Code changes lgtm though
5b6bd8b
to
3f595a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rewards service part looks fine, except I don't understand whether it's okay to remove those extra reset()
s. Everything else also look good to me in general
3f595a8
to
de82723
Compare
de82723
to
a5546b1
Compare
a5546b1
to
d0b245c
Compare
Hey @zenparsing maybe we could give it another try? |
@iefremov I'll rebase and rerun CI today. |
@zenparsing maybe we could finalize this one? We continue to receive crashdumps |
hey @zenparsing why this was closed? The fix is still needed |
Resolves brave/brave-browser#15779
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
,npm run lint
,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: