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

Remove calculation of deterministic id #148

Closed
wants to merge 3 commits into from

Conversation

kaloyan-raev
Copy link
Contributor

// XXX DEPRECATION IN NEXT MAJOR RELEASE
// XXX The determinitic id will be deprecated and the index field will
// XXX become required for any new bucket entries. This is to make sure
// XXX that each bucket entry will have a unique id.
if (index) {
Copy link
Contributor

@braydonf braydonf Mar 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to validate that the index exists, I think this was the plan from before. Will need to test to check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is actually done by calling this isValidIndex() function from the create() function, right? (Line 117 after changes) Or did you mean something different?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think by removing the if statement for if (index), but will need to check that this is okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PATCH method in the bridge will check if there is an index. If there isn't one, it will return Unprocessable Entity Error. So it would be safe to continue creating bucket entries without index. Simply, it won't be possible to rename them afterwards.

However, if you want to force clients to create only bucket entries with index, we should remove the if (index).

@braydonf
Copy link
Contributor

braydonf commented May 7, 2018

There is a minor merge conflict on this in the package.json, FYI

@kaloyan-raev
Copy link
Contributor Author

Resolved.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants