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

Issue 17: Convert ds key to core key #18

Closed
wants to merge 7 commits into from

Conversation

AndrewSisley
Copy link
Contributor

A fairly aggressive conversion of as many ds.Key references as I could to core.Key - including in files operating on the underlying datastores. This is partly because I'm assuming that the cost of core.Key{Key: foo} and core.Key.ToDS() is pretty minimal, and partly because I'm hoping to change core.Key into a more descriptive/safe set of types.

Feel very free to wait on the completion of the core.Key type change before reviewing if you think that this might have a notable negative effect on performance (or if you prefer a few more ds.Key refs for whatever reason).

@AndrewSisley AndrewSisley self-assigned this Oct 25, 2021
@todo
Copy link

todo bot commented Oct 25, 2021

Config logger param package wide

headset: newHeadset(headstore, core.NewKey(id)), //TODO: Config logger param package wide
crdt: crdt,
}
}


This comment was generated by todo based on a TODO comment in 0228369 in #18. cc @sourcenetwork.

@AndrewSisley
Copy link
Contributor Author

Might be wasted effort reviewing this seperately now, will hopefully be putting up the key restructure PR soon (working, some cleanup left to do) which changes the same lines (plus some): https://github.com/sourcenetwork/defradb/tree/merkle-crdt-factory-key-fix-without-store-comp

@AndrewSisley
Copy link
Contributor Author

Replaced by #19 (based off this branch)

@AndrewSisley AndrewSisley deleted the 17-convert-dsKey-to-coreKey branch February 7, 2022 21:06
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.

1 participant