orbit-db-eventstore
orbit-db-eventstore copied to clipboard
orbitdb-archive
Return all entries by default for iterator
I believe, that from my understanding of iterator and the Documentation of iterator,
the default limit should be -1, i.e. returning multiple items, instead of just one.
iterator implies multiple items, so it is counter intuitive to only return 1 Object by default..
This also makes a line more readable, I think, by removing nested ternary operators.
https://github.com/CSDUMMI/orbit-db-eventstore/blob/c43f56515857058cec23e60e8fc8b5ddab9359f2/src/EventStore.js#L54
this is a somewhat large change as it affects the api and should probably at the least cause a minor version update. i would rather this be a change that comes with the 1.0 release.
I'm fine with it being a minor version bump. We typically treat those like major versions anyway. From the semver spec:
Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.
We just need to be really deliberate and explicit in the changelog
@tabcat you do support the idea of this change in general though? Since you are already at version numbers and releasing.
if the orbitdb code using the iterator is using something lower and passing the options directly to it, i would rather not override the defaults of the separate system. otherwise it seems fine, also db._oplog.values gives the same results as what you want to do here.
f the orbitdb code using the iterator is using something lower and passing the options directly to it, i would rather not override the defaults of the separate system. @tabcat what do you mean by this?
if the options and defaults are defined outside of the orbitdb codebase then i wouldnt want to override those defaults. luckily for you they are defined in the store and could easily be changed. i think it would be best to get the authors opinion on this change though since they may have set it to 1 by default for a reason.
@haadcode what do you think of this change? I know that you might not recall, since the blame says, you set this default 5 years ago.
@tabcat I don't think there was any reason, that is still very valid, because it was set 5 years ago.
Not sure if the age of the code is any mark of its validity. Maybe let's just use semver here and classify this as a new major version?