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

(#1767) - get memdown attachments working in browser #2063

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@nolanlawson
Copy link
Member

nolanlawson commented Apr 26, 2014

No description provided.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Apr 26, 2014

The attachment tests pass if you marry this PR with Level/levelup#246. The only tests that don't pass are

  • sync (apparently @adamshih is already on this)
  • migrations (because they don't make sense for memdown; I'll fix them in a later commit)

Other than that, MemDOWN is green as hell in the browser (Chrome).

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Apr 26, 2014

Also this code is terrible, hence WIP. But I will fix in a bit.

@nolanlawson nolanlawson changed the title (#1767) - WIP - get memdown attachments working (#1767) - get memdown attachments working in browser Apr 26, 2014

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Apr 26, 2014

OK, ready. Although the code might still need a refactor; some things are weird (e.g. why the 'md5-' prefix only in the browser?).

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Apr 26, 2014

I'm not sure why memdown is working but level-js and localstorage-down are not. Feel free to run the attachments tests; something is slightly off.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Apr 26, 2014

Yay, memdown attachments are green in Safari and Firefox too. Improbable!

@calvinmetcalf

This comment has been minimized.

Copy link
Member

calvinmetcalf commented Apr 26, 2014

memdown doesn't serialize anything so less to break

@daleharvey

This comment has been minimized.

Copy link
Member

daleharvey commented Apr 28, 2014

e9feb99

@nolanlawson if you have memdown green its probably worth adding to travis, these fixes helped localstorage in chrome, but still broken in firefox

@daleharvey daleharvey closed this Apr 28, 2014

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Apr 28, 2014

Yeah sorry, it's the migration tests that are remaining. It's really just a matter of ignoring those tests unless we're in IDB.

Speaking of which, if we add localStorage to the official release, I guess we'll also want a localStorage -> IDB migration.

@nolanlawson nolanlawson deleted the 1767-memdown-2 branch Apr 29, 2014

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