kriszyp / lmdb-js

Simple, efficient, ultra-fast, scalable data store wrapper for LMDB
Other
505 stars 41 forks source link

lmdb-store vs lmdb docs #94

Closed cekvenich2 closed 2 years ago

cekvenich2 commented 2 years ago

Docs say require('lmdb'), but need to say require('lmdb-store')

In general there is confusion with the packages. One should be marked as deprecated.

kriszyp commented 2 years ago

The docs say:

This library, lmdb-store is published to the NPM package lmdb-store and lmdb

Is there a compelling reason why one should be deprecated (assuming I push the same thing to both npm destinations)?

(FWIW, I will be releasing v2 pretty soon, so this would actually be a relatively good time to switch and make lmdb be the canonical NPM package name, but I actually thought it was more convenient for users to be able to use either name.)

cekvenich2 commented 2 years ago

Yes, the examples don't work! For example on this page: https://www.npmjs.com/package/lmdb-store

This code shown does not work: const { open } = require('lmdb'); // or // import { open } from 'lmdb'; in usage section.

Why does it not work? Because it should say lmdb-store!

kriszyp commented 2 years ago

Heh, well, that's a good point!

kriszyp commented 2 years ago

I think there are a few options/considerations here:

We could just stick with the "lmdb-store" package name, and deprecate "lmdb". However, "lmdb" was recently transferred to me from an old package, and it seems like it would really be nice to move to this clean, simple name as the canonical go-to package for LMDB usage in node (and this would/could also match the reserved entry in the denoland as well, looking forward to when deno is supported).

We could continue to publish to "lmdb" and "lmdb-store". You definitely point out a mistake in the documentation, but of course this alone would be trivial to fix, I can simply note that if you install with "lmdb-store", you must import as that (which is probably already fairly intuitive and obvious anyway).

We could more singularly focus on "lmdb" as the package name. Again, with the upcoming v2.0, this is a good time to focus on the simpler package name. However, I don't think we would want to deprecate the 1.x versions of lmdb-store (or imply deprecation of those versions by deprecating the whole package). A major version release certainly should not demand, expect or require users to immediately upgrade, I would assume 1.x versions will continue to be used for a quite a while, and most everyone uses/downloads 1.x from lmdb-store (and 1.x won't be deprecated for a while).

Perhaps one way to steer this would be add a (text) note on the docs on https://www.npmjs.com/package/lmdb-store that the next major upgrade is found at lmdb@2.0 (rather than a package or 1.x version deprecation), and could also add a simple deprecated and redirecting lmdb-store@2.0 package (with next, not latest tag), for anyone that happens to try "lmdb-store": "2" in package.json.

Another related question would be if the github repository should be renamed. However, I don't think so; within the context of NPM, "lmdb" is unambiguously about a JavaScript package for LMDB, but outside that registry/context, shouldn't be conflated with a fork of LMDB itself.

kriszyp commented 2 years ago

If I were to rename the repository, it might be reasonable to simply call it lmdb-js. With eventually publishing to Deno (and maybe someday adding WASM support), it will be more generally the JS LMDB adapter.

cekvenich2 commented 2 years ago

The name does not matter IMO, what is important that there be one working npm for us to use. Just tell us what one. If you do create a 3rd name, just edit readme on two older npm saying that these are deprecated and to use the 3rd one.

kriszyp commented 2 years ago

Sounds good. I was just curious if anyone had feedback on this. But there won't be any third name (mentioned lmdb-js as a possible repository name, but not npm package name). The npm package name to use is lmdb and will be lmdb going forward (just as the docs indicate with npm install lmdb). I will make this more clear in the docs and versions going forward in v2.0.

cekvenich2 commented 2 years ago

IMO it is OK to have 2 git repo and 2 npm names: as long as the deprecated/legacy ones says so, and it points to the current one.

vladar commented 2 years ago

I agree that moving to a single package name lmdb for 2.0 makes sense, however, I would expect lmdb-store to be still available for 1.x release line, including future maintenance releases (if ever needed).

kriszyp commented 2 years ago

I would expect lmdb-store to be still available for 1.x release line, including future maintenance releases (if ever needed).

Yes, absolutely, lmdb-store@1.x will continue to exist and be maintained (and not deprecated anytime soon).