Make publishes appear immediately in My Files #14

Merged
alexliebowitz merged 5 commits from publish-cache into master 2017-03-28 08:26:46 +02:00
2 changed files with 198 additions and 10 deletions
Showing only changes of commit e523906901 - Show all commits

View file

@ -1,15 +1,103 @@
import lighthouse from './lighthouse.js';
import jsonrpc from './jsonrpc.js';
import {getLocal, setLocal} from './utils.js';
const {remote} = require('electron');
const menu = remote.require('./menu/main-menu');
/**
* Records a publish attempt in local storage. Returns a dictionary with all the data needed to
* needed to make a dummy claim or file info object.
*/
function savePendingPublish(name) {
const pendingPublishes = getLocal('pendingPublishes') || [];
const newPendingPublish = {
claim_id: 'pending_claim_' + name,
txid: 'pending_' + name,
nout: 0,
outpoint: 'pending_' + name + ':0',
name: name,
time: Date.now(),
};
setLocal('pendingPublishes', [...pendingPublishes, newPendingPublish]);
return newPendingPublish;
}
function removePendingPublish({name, outpoint}) {
setLocal('pendingPublishes', getPendingPublishes().filter(
(pub) => pub.name != name && pub.outpoint != outpoint
));
}
/**
* Gets the current list of pending publish attempts. Filters out any that have timed out and
* removes them from the list.
*/
function getPendingPublishes() {
const pendingPublishes = getLocal('pendingPublishes') || [];
const newPendingPublishes = [];
for (let pendingPublish of pendingPublishes) {
if (Date.now() - pendingPublish.time <= lbry.pendingPublishTimeout) {
newPendingPublishes.push(pendingPublish);
}
}
setLocal('pendingPublishes', newPendingPublishes);
return newPendingPublishes
}
/**
* Gets a pending publish attempt by its name or (fake) outpoint. If none is found (or one is found
* but it has timed out), returns null.
*/
function getPendingPublish({name, outpoint}) {
const pendingPublishes = getPendingPublishes();
const pendingPublishIndex = pendingPublishes.findIndex(
({name: itemName, outpoint: itemOutpoint}) => itemName == name || itemOutpoint == outpoint
);
const pendingPublish = pendingPublishes[pendingPublishIndex];
if (pendingPublishIndex == -1) {
return null;
} else if (Date.now() - pendingPublish.time > lbry.pendingPublishTimeout) {
// Pending publish timed out, so remove it from the stored list and don't match
const newPendingPublishes = pendingPublishes.slice();
newPendingPublishes.splice(pendingPublishIndex, 1);
setLocal('pendingPublishes', newPendingPublishes);
return null;
} else {
return pendingPublish;
}
}
function pendingPublishToDummyClaim({name, outpoint, claim_id, txid, nout}) {
return {
name: name,
outpoint: outpoint,
claim_id: claim_id,
txid: txid,
nout: nout,
};
}
function pendingPublishToDummyFileInfo({name, outpoint, claim_id}) {
return {
name: name,
outpoint: outpoint,
claim_id: claim_id,
metadata: "Attempting publication",
};
}
let lbry = {
isConnected: false,
rootPath: '.',
daemonConnectionString: 'http://localhost:5279/lbryapi',
webUiUri: 'http://localhost:5279',
peerListTimeout: 6000,
pendingPublishTimeout: 20 * 60 * 1000,
colors: {
primary: '#155B4A'
},
@ -251,18 +339,49 @@ lbry.getFileInfoWhenListed = function(name, callback, timeoutCallback, tryNum=0)
}, () => scheduleNextCheckOrTimeout());
}
/**
* Publishes a file. The optional fileListedCallback is called when the file becomes available in
* lbry.file_list() during the publish process.
*
* This currently includes a work-around to cache the file in local storage so that the pending
* publish can appear in the UI immediately.
*/
lbry.publish = function(params, fileListedCallback, publishedCallback, errorCallback) {
// Publishes a file.
// The optional fileListedCallback is called when the file becomes available in
// lbry.getFilesInfo() during the publish process.
lbry.call('publish', params, (result) => {
if (returnedPending) {
return;
}
// Use ES6 named arguments instead of directly passing param dict?
lbry.call('publish', params, publishedCallback, errorCallback);
if (fileListedCallback) {
lbry.getFileInfoWhenListed(params.name, function(fileInfo) {
fileListedCallback(fileInfo);
});
}
clearTimeout(returnPendingTimeout);
publishedCallback(result);
}, (err) => {
if (returnedPending) {
return;
}
clearTimeout(returnPendingTimeout);
errorCallback(err);
});
let returnedPending = false;
// Give a short grace period in case publish() returns right away or (more likely) gives an error
const returnPendingTimeout = setTimeout(() => {
returnedPending = true;
if (publishedCallback) {
savePendingPublish(params.name);
publishedCallback(true);
}
if (fileListedCallback) {
savePendingPublish(params.name);
fileListedCallback(true);
}
}, 2000);
//lbry.getFileInfoWhenListed(params.name, function(fileInfo) {
// fileListedCallback(fileInfo);
//});
}
lbry.getVersionInfo = function(callback) {
@ -503,6 +622,60 @@ lbry.showMenuIfNeeded = function() {
sessionStorage.setItem('menuShown', chosenMenu);
};
/**
* Wrappers for API methods to simulate missing or future behavior. Unlike the old-style stubs,
* these are designed to be transparent wrappers around the corresponding API methods.
*/
/**
* Returns results from the file_list API method, plus dummy entries for pending publishes.
* (If a real publish with the same name is found, the pending publish will be ignored and removed.)
*/
lbry.file_list = function(params={}) {
return new Promise((resolve, reject) => {
const {name, outpoint} = params;
/**
* If we're searching by outpoint, check first to see if there's a matching pending publish.
* Pending publishes use their own faux outpoints that are always unique, so we don't need
* to check if there's a real file.
*/
if (outpoint !== undefined) {
const pendingPublish = getPendingPublish({outpoint});
if (pendingPublish) {
resolve([pendingPublishToDummyFileInfo(pendingPublish)]);
return;
}
}
lbry.call('file_list', params, (fileInfos) => {
// Remove any pending publications that are now listed in the file manager
const pendingPublishes = getPendingPublishes();
for (let {name: itemName} of fileInfos) {
if (pendingPublishes.find(() => name == itemName)) {
removePendingPublish({name: name});
}
}
const dummyFileInfos = getPendingPublishes().map(pendingPublishToDummyFileInfo);
resolve([...fileInfos, ...dummyFileInfos]);
}, reject, reject);
});
}
lbry.claim_list_mine = function(params={}) {
return new Promise((resolve, reject) => {
lbry.call('claim_list_mine', params, (claims) => {
// Filter out pending publishes when the name is already in the file manager
const dummyClaims = getPendingPublishes().filter(
(pub) => !claims.find(({name}) => name == pub.name)
).map(pendingPublishToDummyClaim);
resolve([...claims, ...dummyClaims]);
}, reject, reject);
});
}
lbry = new Proxy(lbry, {
get: function(target, name) {
if (name in target) {

15
ui/js/utils.js Normal file
View file

@ -0,0 +1,15 @@
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
/**
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
kauffj commented 2017-03-15 04:24:28 +01:00 (Migrated from github.com)
Review

Should all these functions actually be in the global namespace?

Should all these functions actually be in the global namespace?
alexliebowitz commented 2017-03-15 04:45:51 +01:00 (Migrated from github.com)
Review

In ES6 it defaults to module namespace. That might be an issue if we don't like the calling style (getLocal() vs utils.getLocal()) but it's not going to pollute scope anywhere we don't want it to.

In ES6 it defaults to module namespace. That might be an issue if we don't like the calling style (`getLocal()` vs `utils.getLocal()`) but it's not going to pollute scope anywhere we don't want it to.
* Thin wrapper around localStorage.getItem(). Parses JSON and returns undefined if the value
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
* is not set yet.
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
*/
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
export function getLocal(key) {
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
const itemRaw = localStorage.getItem(key);
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
return itemRaw === null ? undefined : JSON.parse(itemRaw);
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
}
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
/**
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
* Thin wrapper around localStorage.setItem(). Converts value to JSON.
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
*/
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
export function setLocal(key, value) {
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
localStorage.setItem(key, JSON.stringify(value));
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.
}
alexliebowitz commented 2017-03-15 04:24:48 +01:00 (Migrated from github.com)
Review

Possibly break these out into their own module?

Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (utils.savePendingPublish seems really long) but I'm not married to it.

Possibly break these out into their own module? Also, I know this is the first time we've tried exporting functions at the top level instead of using object literals. I like the brevity, especially with long function names like this, (`utils.savePendingPublish` seems really long) but I'm not married to it.
kauffj commented 2017-03-15 04:26:12 +01:00 (Migrated from github.com)
Review

These functions should probably be moved/kept with the LBRY API logic?

These functions should probably be moved/kept with the LBRY API logic?
alexliebowitz commented 2017-03-15 04:28:27 +01:00 (Migrated from github.com)
Review

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?

I don't love "pending publish" as the name for this data structure, kinda wordy. But what else to call it? Publish stub? Dummy publish?
alexliebowitz commented 2017-03-15 04:36:33 +01:00 (Migrated from github.com)
Review

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though.

The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.

Before, we were talking about aiming to make the lbry module only have API wrapper functions, not random utility functions and such. We could still put helper functions like this as private functions that are not exported, though. The main reason I didn't do that is that I felt like lbry.js is already way too big. But it's going to shrink over time, so I guess it's OK.
alexliebowitz commented 2017-03-15 04:43:42 +01:00 (Migrated from github.com)
Review

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn).

If local storage isn't cool, it would be fairly quick to switch to something like nedb.

I know it's awkward to use Local Storage for queryable, list-like data like this. The main alternatives would be IndexedDB (too complicated without a wrapper), Sqlite (possibly too complicated and would require using IPC to do stuff in the main process), or a light JS key/value database like nedb (adds a dependency and stuff to learn). If local storage isn't cool, it would be fairly quick to switch to something like nedb.
kauffj commented 2017-03-15 13:39:03 +01:00 (Migrated from github.com)
Review

The name is fine, probably more important to make it some kind of const if referenced in multiple places.

The name is fine, probably more important to make it some kind of const if referenced in multiple places.
kauffj commented 2017-03-15 13:39:32 +01:00 (Migrated from github.com)
Review

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.

I think lbry is the right place for this. They change the behavior of API calls provided by lbry.js.
kauffj commented 2017-03-15 13:39:45 +01:00 (Migrated from github.com)
Review

Local storage is fine.

Local storage is fine.