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

chore: Make Cid versions consistent #57

Merged
merged 5 commits into from
Nov 24, 2021

Conversation

AndrewSisley
Copy link
Contributor

@AndrewSisley AndrewSisley commented Nov 24, 2021

Closes #49

Review contains some small tweaks/fixes, followed by a proposed update to modifiy all our non-dockey Cids to v1 (from v0). Dockeys are already using v1 Cids (I think).

This will impact existing databases considerably and doubt we can rely on an existing migrations (such as when updating badger versions). I do not know if we need to actively do anything for this, or if our current users are happy to just nuke and rebuild their databases - please let me know if more is required here.

To do:

  • Either rename or drop 'PROPOSAL' commit depending on other's input

Code is dead, and untested - consider removal (is half used by a single unit which only counts the keys, but doesn't assert their values)
@AndrewSisley AndrewSisley added area/crdt Related to the (Merkle) CRDT system refactor This issue specific to or requires *notable* refactoring of existing codebases and components labels Nov 24, 2021
@AndrewSisley AndrewSisley self-assigned this Nov 24, 2021
@AndrewSisley AndrewSisley linked an issue Nov 24, 2021 that may be closed by this pull request
@AndrewSisley AndrewSisley force-pushed the sisley/chore/I49-investigate-cid-usage branch from 44e0dd9 to 36b5856 Compare November 24, 2021 19:31
Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

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

All LGTM 👍

side note: I'm always amused seeing sprinkled in your PRs, a trickle effect of slowly removing dead and commented out legacy code

@AndrewSisley
Copy link
Contributor Author

Nice, will rename the commit and then merge

RE: sidenote - definitely prefer dripping stuff in as opposed to the hassle/risk of doing it all at once (although it doesn't play as nicely with squashing commits on merge sadly)

@AndrewSisley AndrewSisley force-pushed the sisley/chore/I49-investigate-cid-usage branch from 36b5856 to c909037 Compare November 24, 2021 23:40
@AndrewSisley AndrewSisley merged commit d764c4b into develop Nov 24, 2021
@AndrewSisley AndrewSisley deleted the sisley/chore/I49-investigate-cid-usage branch November 29, 2021 19:39
@orpheuslummis orpheuslummis mentioned this pull request Jan 29, 2022
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
* Rename test file to go runs tests

* Remove commented out code

* Remove unused property

* Return V0 Cids from blockstore.AllKeysChan

Code is dead, and untested - consider removal (is half used by a single unit which only counts the keys, but doesn't assert their values)

* Use v1 Cids throughout the codebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/crdt Related to the (Merkle) CRDT system refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate Cid versioning story
2 participants