Change Chrome API design
AcceptedPublic

Authored by alex on Sun, May 17, 4:14 PM.

Details

Reviewers
broulik
ngraham
meven
Group Reviewers
Plasma
Summary
  • The FindProfile class was only for Chrome implemented => removed
  • Having ProfileBookmarks and Profile class is unnecessary, especially because they are only needed for Chrome
  • Better error handling
Test Plan

Should work as before

Diff Detail

Repository
R120 Plasma Workspace
Branch
api_inconsistencies (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27066
Build 27084: arc lint + arc unit
alex created this revision.Sun, May 17, 4:14 PM
Restricted Application added a project: Plasma. · View Herald TranscriptSun, May 17, 4:14 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
alex requested review of this revision.Sun, May 17, 4:14 PM
anthonyfieroni added a subscriber: anthonyfieroni.

Commented code should be removed.

runners/bookmarks/browsers/chromeprofile.h
12

You can move contructor to private section, that will make all things clear, findAllProfiles should be the only one allowed creator.

alex updated this revision to Diff 83028.Sun, May 17, 6:08 PM

Make constructor private

alex updated this revision to Diff 83029.Sun, May 17, 8:10 PM
alex marked an inline comment as done.

Create test cases

alex updated this revision to Diff 83116.Fri, May 22, 3:35 PM
alex retitled this revision from WIP: Change Chrome API design to Change Chrome API design.
alex edited the summary of this revision. (Show Details)
alex added reviewers: ngraham, meven.

Change import, formatting

meven added a comment.Fri, May 22, 4:56 PM

Seems good, two small clean ups

runners/bookmarks/browsers/chromeprofile.cpp
9

{ on new line

28

Remove

alex updated this revision to Diff 83118.Fri, May 22, 5:02 PM

Formatting, remove debug statement

alex marked 2 inline comments as done.Fri, May 22, 5:02 PM
meven accepted this revision as: meven.Sat, May 23, 2:04 PM

Two smalls things.

Seems good to me otherwise.

Give some times to other to have a final say before merging.

runners/bookmarks/browsers/chromeprofile.cpp
34

Perhaps use an iterator instead of copying the keys.

runners/bookmarks/fetchsqlite.cpp
32

Is this used here ?

This revision is now accepted and ready to land.Sat, May 23, 2:04 PM
alex updated this revision to Diff 83131.Sat, May 23, 3:56 PM

Use iterator, remove unused include

alex marked 2 inline comments as done.Sat, May 23, 3:57 PM
meven accepted this revision.Wed, Jun 3, 7:24 AM

@broulik perhaps a review ?