Skip to content
This repository has been archived by the owner on Sep 30, 2023. It is now read-only.

feat: add the optional argument cache to the constructor #41

Closed
tmorin opened this issue Oct 25, 2019 · 4 comments
Closed

feat: add the optional argument cache to the constructor #41

tmorin opened this issue Oct 25, 2019 · 4 comments

Comments

@tmorin
Copy link
Contributor

tmorin commented Oct 25, 2019

Hi,

Could it be possible to provide the instance of the cache to use as an argument of the constructor as below?

const keystore1 = new Keystore()
const keystore2 = new Keystore(undefined, new LRU(10))
const keystore3 = new Keystore(level('./somewhere'), new LRU(10))
const keystore4 = new Keystore(level('./somewhere'))

Or using an options object?

const keystore1 = new Keystore()
const keystore2 = new Keystore({
  cache: new LRU(10)
})
const keystore3 = new Keystore({
  store: level('./somewhere'),
  cache: new LRU(10)
})
const keystore4 = new Keystore({
  store: level('./somewhere')
})
// for compatibilities
const keystore4bis = new Keystore(level('./somewhere'))

If you are agree I can do the pull request with the additional tests.

Best regards

@aphelionz
Copy link
Contributor

What do you think @shamb0t and @haadcode

@shamb0t
Copy link
Contributor

shamb0t commented Oct 28, 2019

This would be a nice addition, thank you @tmorin! Though it's a breaking change I would prefer the options object approach to avoid passing undefined in the args, what do you think?

@tmorin
Copy link
Contributor Author

tmorin commented Oct 28, 2019

hi @shamb0t , @aphelionz
The options' object is my preference too.
Best regards

@shamb0t
Copy link
Contributor

shamb0t commented Nov 7, 2019

closed by #42, thanks @tmorin!

@shamb0t shamb0t closed this as completed Nov 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants