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

Update memdown to v1.4.1 and update test suite #5

Merged
merged 6 commits into from
Nov 20, 2017

Conversation

staltz
Copy link
Contributor

@staltz staltz commented Nov 13, 2017

This commit make this package compliant with the latest Level ecosystem,
using memdown 1.4.1 (which uses abstract-leveldown v2.7). It is an
overhaul due to how memdown uses Red Black Trees as internal data
structure, this package had to adapt in order to (de)serialize those
trees.

The test suite comes from the standard abstract-leveldown test suite,
using tape as test runner.

This commit brings make this compliant with the latest Level ecosystem,
using memdown 1.4.1 (which uses abstract-leveldown v2.7). It is an
overall due to how memdown uses Red Black Trees as internal data
structure, this package had to adapt in order to (de)serialize those
trees.

The test suite comes from the standard abstract-leveldown test suite,
using tape as test runner.
@toolness
Copy link
Owner

Oh thanks! I'll take a look at this soon!

var niceStringify = require('./nice-stringify');
function serializeStore(store) {
var result = {};
Object.keys(store).forEach(location => {

Choose a reason for hiding this comment

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

The tests are failing on Travis because it's running with Node 0.10.0. You should either update the .travis.yml file to a later Node version or remove the arrow functions (this one and the one on line 11)

Copy link
Owner

Choose a reason for hiding this comment

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

Oh geez, I wrote this a billion years ago. I'm fine with upgrading the version of Node that this package expects--I assume this would involve not only updating .travis.yml, but also adding something to package.json indicating the version of node that's needed?

@toolness
Copy link
Owner

This PR looks nice to me, though I'm unfamiliar with memdown's use of Red Black Trees. As long as Travis is updated to run the tests properly, and package.json is updated to reflect the minimum version of node that this package now requires, I'm good with it!

@emschwartz
Copy link

When this is merged it would be good to update https://github.com/Level/awesome/#stores as well to indicate that jsondown is up to date with 2.7

@staltz
Copy link
Contributor Author

staltz commented Nov 20, 2017

Updated the Travis config to use a long-term support version of Node.js. :)

@toolness
Copy link
Owner

Woot thanks!!

I'm going to merge this now, but I think it still needs to be published on npm, right? I haven't done that in a really long time but I will give it a shot. Alternatively, @staltz or @emschwartz, are either of you interested in becoming maintainers of this project? No worries if not, I figured I might as well ask.

@toolness toolness merged commit c8b9a91 into toolness:master Nov 20, 2017
@emschwartz
Copy link

Up to you, you can add me as a maintainer if you'd like and I can publish it to NPM

@staltz
Copy link
Contributor Author

staltz commented Nov 20, 2017

Hi! Thanks for merging. I can also be a maintainer. In fact I already published my fork as the package @staltz/jsondown but it would be better for the community if there isn't a divergence, no need to choose between the original package or the fork package.

@emschwartz
Copy link

emschwartz commented Nov 21, 2017

@toolness can you make @staltz a maintainer so he can update the package on NPM? Thanks!

(Also, @staltz, when you bump the version I think it should be 1.0.0 rather than 1.1.2)

@toolness
Copy link
Owner

Sure thing, just added you @staltz !

@staltz
Copy link
Contributor Author

staltz commented Nov 21, 2017

Version 1.0.0 released! :)

@staltz staltz deleted the update-abs-leveldown branch November 21, 2017 21:44
emschwartz added a commit to emschwartz/awesome that referenced this pull request Jan 14, 2018
vweevers pushed a commit to Level/awesome that referenced this pull request Jan 14, 2018
bestsoftwaretopappreviews44 added a commit to bestsoftwaretopappreviews44/awesome that referenced this pull request Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants