Closed
Bug 1159561
Opened 9 years ago
Closed 9 years ago
Rework history storage to support syncing: initial cleanup
Categories
(Firefox for iOS :: Data Storage, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: rnewman, Assigned: rnewman)
References
Details
Attachments
(1 file)
No description provided.
Assignee | ||
Comment 1•9 years ago
|
||
The `Deferred` switch was way cleaner than I expected! Whichever of you gets to this first wins. Go! :D
Attachment #8599064 -
Flags: review?(wjohnston)
Attachment #8599064 -
Flags: review?(sarentz)
Attachment #8599064 -
Flags: review?(nalexander)
Comment 2•9 years ago
|
||
Comment on attachment 8599064 [details] [review] Pull request. In general, I'm fine with this -- questions oand comments on GH. However, I do not understand (and haven't read the further sync stuff /to/ understand) what the desired Visit/SiteVisit/something else split is. I find that confusing and unexpected and suggest wesj weigh in tomorrow and I'll loop back as well.
Attachment #8599064 -
Flags: review?(nalexander) → review+
Assignee | ||
Comment 3•9 years ago
|
||
I commented in the PR, but in short: `SiteVisit` is what the frontend code was already consuming, just with a different name. `Visit` is the simpler 'root' that Sync will produce. This is effectively the alternative to the `RemoteTab`/`Tab` issue that you commented on in tab sync, and it'll get resolved (likely to the point of just killing `SiteVisit` altogether) as I move forward with syncing and rip out the existing history storage and UI-facing bits. After all, we don't show visits in the UI…
Assignee | ||
Comment 4•9 years ago
|
||
I suspect we'll run into this "annotated version" (anti?)-pattern a few times: that is, we have these domain objects that end up being decorated with row IDs, foreign key references, etc. IMO that'll frequently often be a sign that there's "model-view-controller smear" going on: rather than the model exposing domain items and what the UI wants (e.g., URLs and titles by frecency) it exposes the database contents (a cursor of objects containing row IDs). That viewpoint is why I expect SiteVisit to die -- the reference from a visit to a site is an implementation detail of the database schema.
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #4) > frequently often… frequently often sometimes always. This is how my brain works when I wake up at 4:30am.
Comment 6•9 years ago
|
||
Comment on attachment 8599064 [details] [review] Pull request. Love the new Deferred style vs blocks.
Attachment #8599064 -
Flags: review?(sarentz) → review+
Assignee | ||
Updated•9 years ago
|
Attachment #8599064 -
Flags: review?(wjohnston)
Assignee | ||
Comment 7•9 years ago
|
||
https://github.com/mozilla/firefox-ios/commit/26bb5088ba94667753d9f0ebbe3d9127e1149b0f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 8•9 years ago
|
||
rnewman, can you ping me on this stuff in the future? I wrote this stuff. I like to keep some tabs on changes in it...
Comment 9•9 years ago
|
||
Oh you did! I just got removed....
You need to log in
before you can comment on or make changes to this bug.
Description
•