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

Add support for binary data to localStorage #54

Closed
wants to merge 7 commits into from

Conversation

jviereck
Copy link
Contributor

This fixes half of #40 - WebSQL support is outstanding and this is why the test is failing at this point

@@ -6,7 +6,7 @@
// a prompt.
var DB_SIZE = 5 * 1024 * 1024;
var DB_VERSION = '1.0';
var SERIALIZED_MARKER = '__lfsc__:';
var SERIALIZED_MARKER = '__lfsc__';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. This change was not intended. Reverted the change again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! :-)

Reverse accidental change of SERIALIZED_MARKER back again.
@tofumatt
Copy link
Member

(PS: Don't worry too about this not merging cleanly; I'll be sure to add my WebSQL code as another commit in this branch to get the tests working and merge it in by hand.)

@jviereck
Copy link
Contributor Author

(PS: Don't worry too about this not merging cleanly; I'll be sure to add my WebSQL code as another commit in this branch to get the tests working and merge it in by hand.)

Do you have your current WebSQL code online already to have a look at it?

@tofumatt
Copy link
Member

No, I’m currently working on it… I’m hoping it’ll be ready today though.

@tofumatt
Copy link
Member

(This is taking longer than expected, and I'm adding a bunch of tests to this as well. I'm hoping this week sometime.)

@jviereck
Copy link
Contributor Author

No worries. As soon as you have something to look at, I am happy to do
so. Also, feel free to send a PR my way if you want ;)

On 18/02/14 02:32, Matthew Riley MacPherson wrote:

(This is taking longer than expected, and I'm adding a bunch of tests
to this as well. I'm hoping this week sometime.)


Reply to this email directly or view it on GitHub
#54 (comment).

@creasy1
Copy link

creasy1 commented Feb 20, 2014

True nube here mea culpa. Still in the dark. I have made the candle. In need of the fire to light my path to illumination.

@tofumatt
Copy link
Member

Pull request poetry, it's all the rage!

@creasy1
Copy link

creasy1 commented Feb 20, 2014

Having the gain the equivalent to 1 candlepower of knowledge in the world of code write. I now I'm in need of more like an Epa-candlepower light. 1 x 10e18 more candlesticks before I could begin (forgive the cliche) "To see the Forrest through the trees"

@tofumatt
Copy link
Member

I'm going to close this branch as the code is in #73 and that's what will get merged.

@tofumatt tofumatt closed this Mar 14, 2014
@creasy1
Copy link

creasy1 commented Mar 18, 2014

Git account closed in way over my head couldn't justify paying for
something I couldn't use.
On Mar 14, 2014 6:44 PM, "Matthew Riley MacPherson" <
notifications@github.com> wrote:

I'm going to close this branch as the code is in #73https://github.com/mozilla/localForage/pull/73and that's what will get merged.

Reply to this email directly or view it on GitHubhttps://github.com//pull/54#issuecomment-37705099
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants