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

new release? #301

Closed
ralphtheninja opened this issue Jan 16, 2015 · 6 comments
Closed

new release? #301

ralphtheninja opened this issue Jan 16, 2015 · 6 comments

Comments

@ralphtheninja
Copy link
Member

Do we need to change anything in node-levelup in order to make it work with node-leveldown@1.0.1?

It would be nice to release a new version of node-levelup -> so level-packager can be updated -> so level can be updated as well.

/cc @rvagg

@rvagg
Copy link
Member

rvagg commented Jan 17, 2015

This is causing too much pain for everyone, including me. I'm going to abort on the remove-writestream changes that are currently blocking and we'll do a levelup@1.0.0 with what's currently in that branch I think, I'll try and propose a tip for that today and we can get agreement on release.

@ralphtheninja
Copy link
Member Author

I'm trying out levelup with leveldown@1.0.0 right now and have some failing tests. I could take a look and try to solve them if you like. You would probably do it quicker than me though, but I'd like to get more involved into this project now so it would be good practice for me.

@ralphtheninja
Copy link
Member Author

For some reason this doesn't throw exception (in test/batch-test.js):

assert.exception(this.batch.put.bind(this.batch, 'foo1'), function (err) {
  console.log('err.name', err.name, 'err.message', err.message)
  if (err.name != 'WriteError')
    return false
  if ('value cannot be `null` or `undefined`' != err.message)
    return false
  return true
})

But this clearly does:

var levelup = require('./')
var db = levelup('dbb')
var batch = db.batch()
batch.put.bind(batch, 'foo1')()

@ralphtheninja
Copy link
Member Author

Oh, I suspect this is related to abstract-leveldown not throwing an error if the value is not defined in a batch.put operation. I'm sure @rvagg knows exactly why this is happening.

@ralphtheninja
Copy link
Member Author

Think I understand why this is happening now. Feel free to correct me.

In abstract-chained-batch.js from abstract-leveldown@2.0.3

AbstractChainedBatch.prototype.put = function (key, value) {
  this._checkWritten()

  var err = this._db._checkKey(key, 'key', this._db._isBuffer)
  if (err)
    throw err

  if (!this._db._isBuffer(key)) key = String(key)
  if (!this._db._isBuffer(value)) value = String(value)

  if (typeof this._put == 'function' )
    this._put(key, value)
  else
    this._operations.push({ type: 'put', key: key, value: value })

  return this
}

We no longer throw an exception if value is undefined or null, since that was removed in this commit: Level/abstract-leveldown@66254df

@rvagg Should these failing tests in node-levelup be removed or altered? I guess you had a reason for not throwing errors for these values.

@ralphtheninja
Copy link
Member Author

Updated node-leveldown and tests here #302

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

No branches or pull requests

2 participants