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 apiVersion from 720 to 730 #86

Merged
merged 5 commits into from
Feb 18, 2025
Merged

Conversation

nickchomey
Copy link

@nickchomey nickchomey commented Feb 18, 2025

This resolves #85.

version 7.2 / 720 is experimental, for internal testing purposes only. Surely everyone is using v7.3 - which is the only fully-supported version of foundationdb.

Without this change, we cannot pass version 730 to setAPIVersion().

I've also updated the github workflows, readme and changelog to reflect this (and a new 2.0.2 release)

@josephg
Copy link
Owner

josephg commented Feb 18, 2025

Thanks! IT'd be good to re-generate the options as well, though maybe nothing has changed.

@josephg josephg merged commit 0dcc027 into josephg:master Feb 18, 2025
0 of 3 checks passed
@keijokapp
Copy link

To avoid redundant work with releasing, chores etc...

I've been maintaining my fork (keijokapp/node-foundationdb, @arbendium/foundaiondb). Aside from other changes, there's also a significant amount of maintenance work done and maybe it makes sense to incorporate some of that here. For example, I see that Node version was updated but there's a lot of leftover legacy code for versions that have long been unsupported. Also fixed the CI. Ideally, of course, I'd like to get most of the changes made in that fork here. Ideally, I'd like to maintain this library myself.

@nickchomey
Copy link
Author

@keijokapp Your work looks extremely valuable! I hope that it can get merged into here somehow.

Though, since there are breaking changes, can I suggest making a brief guide on how to migrate from the current format to the new one?

@keijokapp
Copy link

Though, since there are breaking changes, can I suggest making a brief guide on how to migrate from the current format to the new one?

All user-facing changes are in the change log. There isn't anything too drastic.

I added the disclaimer to README due to the following change:

Changed range operation semantics. All range methods (except the "native" or "raw" ones) have a StartsWith variant. The start and end arguments of the original methods are optional and default to subspace start and end (inclusive) respectively.

because, as described in #78, in a worst case, migrating without any consideration/testing/etc may result in a hard-to detect data loss. Basically, make sure that you don't call getRange (&family) and clearRange with second argument being undefined or null before migrating. Use StartsWith variants instead. I initially kept full backwards compatibility with the upstream using one of the methods described in that issue, but due to inactivity here I decided that hard fork and incompatible changes are warranted. If I were able to merge those here, I would likely re-add a safety feature.

All the changes are more or less about subtleties. It's difficult, if not impossible to know which subtleties other user might rely on. I considered marking some changes as backwards-incompatible, but all of them can cause issues if the code is awkward enough. For example, the change about every transaction run getting a completely new transaction object. I wouldn't expect users to rely on the objects' referential equality between the runs but it's not specifically disallowed either. The change was made to avoid bugs, potentially one of which I found in the directory layer.

@josephg
Copy link
Owner

josephg commented Feb 19, 2025

Ideally, of course, I'd like to get most of the changes made in that fork here. Ideally, I'd like to maintain this library myself.

@keijokapp: Wanna do that?

What I might do is:

  • Link to your github project from the README here
  • Archive this project
  • Give you permission to publish to foundationdb on npm

Then if you publish your changes into a new major version of the npm library, people can migrate when they want to.

How does that sound?

@keijokapp
Copy link

@keijokapp: Wanna do that?

Absolutely. Thanks!

Link to your github project from the README here

Wouldn't it be better to transfer the ownership? So that issues etc would come along and redirecting works automatically. Idk, what's the best practice. I would discard (or overwrite) my current fork anyway. I haven't decided whether I want to keep it under my personal GH namespace or "arbendium" (which I also fully control but is less personal). Keeping it under your namespace would also be an option for me I guess.

I'm having unusally busy times right now so I can't make any promises regarding when I can make a release.

My NPM username is also keijokapp.

@nickchomey
Copy link
Author

My two cents is that it would be ideal if @keijokapp could get admin permissions/ownership of this repo and the npm library, so that no one has to do anything - they just keep using what they already have. And it would maintain the issues, prs, history/lineage, credit to @josephg etc...

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.

Having difficulty with apiVersion
3 participants