-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat(core) replace config.Config with repo.Repo #578
Conversation
2a9559d
to
df4d122
Compare
LGTM but see failing tests |
failing test is ok github.com/jbenet/go-ipfs/util 0.228s Should we activate |
Yeah, sure. sigh timing. @briantigerchow and https://build.protocol-dev.com/job/network-test/930/console |
624a3f3
to
68c9986
Compare
Ah thanks. Missed that. Thank goodness for online tests. |
All tests pass, but I'd like to read through one more time before merging. Mind if I merge this tomorrow? |
@briantigerchow 👍 no rush |
d0f15c7
to
c457a84
Compare
@whyrusleeping @jbenet does passing this argument effectively instruct the dagbuilder to pin recursively?
@jbenet wasn't sure about the terminology here. I'm certain what I wrote is not entirely correct. Feel free to edit.
This declarative style is simpler to compose than the imperative wiring up of objects. + pass context to StartOnlineServices as parameter. one by one, trying to remove dependencies on node state so these initialization steps can be broken down.
@jbenet cool with this?
this is tested pretty thoroughly, but it's changes are cross-cutting so I still worry about regressions. Going to merge now. Let me know if you experience any strange issues around |
feat(core) replace config.Config with repo.Repo
chore: linting fixes
closes #376 RFCR @jbenet