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

Allow two delete device records #1125

Merged
merged 3 commits into from
Feb 15, 2019
Merged

Allow two delete device records #1125

merged 3 commits into from
Feb 15, 2019

Conversation

AlexeyBarabash
Copy link
Contributor

@AlexeyBarabash AlexeyBarabash commented Dec 18, 2018 •
edited
Loading

Addresses brave/brave-browser#2817

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

This is more for DCHECK sanity. Impossible to test on release builds.
npm run test -- brave_unit_tests --filter=BraveSyncServiceTest.OnDeleteDevice

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

@AlexeyBarabash AlexeyBarabash self-assigned this Dec 18, 2018
@AlexeyBarabash AlexeyBarabash force-pushed the sync_allow_two_delete_device_records branch 2 times, most recently from 107dbd6 to ae520e8 Compare January 4, 2019 15:21
@AlexeyBarabash AlexeyBarabash changed the title Allow two delete device records; addresses browser-android-tabs#933 Allow two delete device records Jan 4, 2019
@AlexeyBarabash
Copy link
Contributor Author

Created a new issue for this PR brave/brave-browser#2817 .

@AlexeyBarabash AlexeyBarabash force-pushed the sync_allow_two_delete_device_records branch from ae520e8 to a826eeb Compare January 14, 2019 10:06
@AlexeyBarabash
Copy link
Contributor Author

Rebased to current master.

darkdh
darkdh previously approved these changes Feb 13, 2019
View reviewed changes
@AlexeyBarabash AlexeyBarabash force-pushed the sync_allow_two_delete_device_records branch from a826eeb to 39500cd Compare February 14, 2019 11:57
@AlexeyBarabash
Copy link
Contributor Author

Rebased to current master again and fixed lint issues.
@darkdh could you approve it again?

@AlexeyBarabash AlexeyBarabash force-pushed the sync_allow_two_delete_device_records branch from 39500cd to b74cb87 Compare February 14, 2019 16:30
@AlexeyBarabash AlexeyBarabash merged commit 6ca6a97 into master Feb 15, 2019
@AlexeyBarabash AlexeyBarabash mentioned this pull request Mar 6, 2019
Closed
@AlexeyBarabash AlexeyBarabash added this to the 0.62.x - Beta milestone Mar 14, 2019
@mihaiplesa mihaiplesa deleted the sync_allow_two_delete_device_records branch May 13, 2019 10:55
@stephendonner stephendonner mentioned this pull request Jun 10, 2022
Merged
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darkdh darkdh darkdh approved these changes

@bridiver bridiver Awaiting requested review from bridiver

Assignees

@AlexeyBarabash AlexeyBarabash

Labels
enhancement feature/sync
Projects
Sync - All platforms
Archived in project
Milestone
0.62.x - Release
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexeyBarabash @darkdh @NejcZdovc