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

Refactoring #31

Open
7 of 9 tasks
thiagodelgado111 opened this issue Sep 4, 2018 · 3 comments
Open
7 of 9 tasks

Refactoring #31

thiagodelgado111 opened this issue Sep 4, 2018 · 3 comments

Comments

@thiagodelgado111
Copy link
Contributor

thiagodelgado111 commented Sep 4, 2018

TODO

  • Move Cache to the store
  • Throw errors if a required argument isn't passed to the store
  • Remove .id property since it's the same as the address
  • Make the Store extend EventEmitter
  • Clean up replication event handling
  • Move snapshot related code to its own space
  • Remove Index from Store
  • Should we rename Index to IndexingStrategy?
  • Should Cache be named like that since it feels it does more than just caching data?
@haadcode
Copy link
Member

haadcode commented Sep 4, 2018

Thank you for opening the issue @thiagodelgado111! 👍 For context, we've discussed this with @thiagodelgado111 in private as to what the plan is.

Add "store builder"

Can you elaborate on this? If I remember correctly, we discussed not adding this, but it may not be what you mean here.

Should Cache be named like that since it feels it does more than just caching data?

Let's keep Cache for now. While you're correct that it does "more" than just "caching" (that is, it persists a state on disk), from the database's perspective it's a cache (as in, we don't use it as a database per se).

All others LGTM and as discussed 👍

@thiagodelgado111
Copy link
Contributor Author

thiagodelgado111 commented Sep 4, 2018

Thanks for checking it out! About the store builder, that must have sneaked into my notes somehow, removed it! (BTW, to be clear, this is the StoreBuilder https://gist.github.com/thiagodelgado111/9103a5d024d3c06b37be4b6f03baf8bc)

@aphelionz
Copy link
Contributor

Hi @thiagodelgado111! With regards to the last two unchecked items - how are those going/ Might I be able to assist in any way?

  • Remove Index from Store
  • Should we rename Index to IndexingStrategy?

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 a pull request may close this issue.

3 participants