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

[iOS-Sync] - Bookmarks API Encapsulation #9145

Merged
merged 3 commits into from
Jun 18, 2021
Merged

Conversation

soner-yuksel
Copy link
Contributor

@soner-yuksel soner-yuksel commented Jun 15, 2021

The usage of how constructors in BookmarksAPI and BookmarksModelListenerImp is aligned with History counterpart.

The usage of singleton instances are removed and API is initialized with assigning the BookmarkModel and BookmarkUndoService directly without using the browserState directly. Also the initializer used privately and moved to a private header file.
In addition the Bookmark Model Listener is exposed publicly anymore and moved to a private file.

Resolves brave/brave-ios#3802

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

@soner-yuksel soner-yuksel added CI/skip-android Do not run CI builds for Android CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 labels Jun 15, 2021
@soner-yuksel soner-yuksel force-pushed the ios/bookmarks-api-change branch from 5152cb7 to 6594f0d Compare June 16, 2021 16:20
@soner-yuksel soner-yuksel changed the title [WIP] [iOS-Sync] - Bookmarks API Encapsulation [iOS-Sync] - Bookmarks API Encapsulation Jun 16, 2021
@soner-yuksel soner-yuksel marked this pull request as ready for review June 16, 2021 16:33
@soner-yuksel soner-yuksel force-pushed the ios/bookmarks-api-change branch from e00b35e to 4f27a3d Compare June 16, 2021 20:30
@Brandon-T Brandon-T merged commit c3a0062 into master Jun 18, 2021
@Brandon-T Brandon-T deleted the ios/bookmarks-api-change branch June 18, 2021 13:21
@soner-yuksel soner-yuksel added this to the 1.27.x - Beta milestone Jun 18, 2021
soner-yuksel added a commit that referenced this pull request Jun 21, 2021
soner-yuksel added a commit that referenced this pull request Jun 21, 2021
@soner-yuksel soner-yuksel restored the ios/bookmarks-api-change branch May 31, 2022 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/skip-android Do not run CI builds for Android CI/skip-macos-x64 Do not run CI builds for macOS x64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Aligning Bookmarks API with history - Lifetime Observation and Constructor Encapsulation
3 participants