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

node: use Repository on the Node and slightly improve testing utilites #94

Merged
merged 6 commits into from
Sep 29, 2021

Conversation

Wondertan
Copy link
Member

Context

This is the last TODO of #86.

Changes

  • MockRepository for Node
  • Group mocks in node and core packages into testing.go
  • Split DefaultConfig into default for Light and Full. We don't have a difference right now that will change soon.
  • Actually use Repository on the node and tests update

…l them in one place and (2)make them importable. Unfortunately those testing functions will be part of the build
…ests; remove old in-memory Datastore and put ConfigLoader in the Node instead of just Config
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

I don't really get why we need the LoadConfig stuff - makes things more complicated for no reason. Let's just load the config during initialisation and store the pointer to the config on the Node please. I really want the Node init logic to be as clear and possible.

core/testing.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

lgtm

node/node.go Outdated Show resolved Hide resolved
Wondertan and others added 2 commits September 29, 2021 15:49
Co-authored-by: rene <41963722+renaynay@users.noreply.github.com>
@renaynay renaynay self-requested a review September 29, 2021 13:22
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

LGTM - resolved in sync w @Wondertan

@Wondertan Wondertan merged commit ffe11bc into hlib/node-repo Sep 29, 2021
@Wondertan Wondertan deleted the hlib/node-repo-usage branch September 29, 2021 13:24
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.

3 participants