Closed lewizho99 closed 7 years ago
Hi, did you happen to coordinate this with @poltak? We've been discussing approaches in #29.
@treora, i talked with @oliversauter a while ago and i got in touch with @poltak when i was almost through but for now the bookmark import is very analogous to the history import. That is it gets the bookmarks convert each bookmark item to a page doc and a visit doc then save into the db
Sounds similar to #47. @poltak and I have been discussing the overall approach to importing bookmarks and history. He is getting into this while I am trying to focus on other things, so he'll be the one to talk to.
i am a bit confused here because i talked to @poltak before working on it and imidiately i finished and 1), @poltak told me to talk to you @Treora about this PR as he is busy with another module 2), so what happens to this PR?
@lewizho99: I didn't say talk to him because I am busy. I simply stated that he may also be interested in it. Right after I said that I'll give you some feedback on it (this PR) by tomorrow.
I haven't forgotten about you, just gotta sit tight until I can get you some feedback on this 😉
ah cool, hoping to hear from you soon... thanks for the quick response hoping to hear from you.
@lewizho99: Thanks for helping out. First off, I'd recommend getting familiar with #29 and all the discussion in that thread. Most of the things I can see wrong in this PR so far are related to existing discussion in there. I'll put links to previous discussion in code comments.
The main issue with this so far is that we haven't decided on an actual structure for representing our bookmarks yet. Here I can see you're reusing the page
and visit
docs from import-history
, but we were thinking of a third type of doc, bookmark
, to use with import-bookmarks
(discussion here, here). As far as I can see, the logic in this PR is the same as import-history
, just using a different source (browser.bookmarks
vs browser.history
).
Other minor things:
console.log
calls, code with comments above saying "ignore this"/"test", etc); git commit patch (git commit -p
) is great for this, giving you an interactive mode to choose which lines to stage in a commit (or try a good graphical git client that allows you to easily stage only parts of your work, like Tower)git commit -p
helps here), although not everyone works that way. If there's more commit information, I can more easily see your thought process and how your PR changes over time (here's some good guidelines)Closing due to inactivity. A more likely implementation for this feature is being made by @poltak over at WorldBrain (see https://github.com/WorldBrain/WebMemex/pull/25).
The Bookmark Import is ready but not yet called similar to the History Import.