Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

For #3967 - Invokes pending deletion with more bookmark actions #4522

Merged
merged 1 commit into from Aug 21, 2019

Conversation

ekager
Copy link
Contributor

@ekager ekager commented Aug 2, 2019

We were only doing this in onPause but I think it's too late by then. In my testing, this fixed the known issues but will need QA to try to find other points

Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry or does not need one
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

@ekager ekager requested a review from a team as a code owner August 2, 2019 22:26
Copy link
Contributor

@colintheshots colintheshots left a comment

Choose a reason for hiding this comment

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

Looks good, but I think 90% of these cases are covered by calling invokePendingDeletion onStop. If we have to call it individually for each action, then it will likely get missed when we add new actions for new features.

@sblatz
Copy link
Contributor

sblatz commented Aug 16, 2019

@ekager if you fix the merge conflicts for this we can get it merged in!

@boek boek merged commit c1b1c2f into mozilla-mobile:master Aug 21, 2019
@ekager ekager deleted the pending-bookmarks branch August 21, 2020 20:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants