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

Load chain config from file #5694

Merged
merged 59 commits into from
May 5, 2020
Merged

Load chain config from file #5694

merged 59 commits into from
May 5, 2020

Conversation

shayzluf
Copy link
Contributor

Feature

What does this PR do? Why is it needed?
load chain config from yaml file
Which issues(s) does this PR fix?

Part of #5693

@shayzluf shayzluf requested a review from a team as a code owner April 30, 2020 15:18
shared/cmd/flags.go Outdated Show resolved Hide resolved
@shayzluf shayzluf requested a review from nisdas April 30, 2020 16:28
@shayzluf shayzluf added the Ready For Review A pull request ready for code review label Apr 30, 2020
shared/cmd/flags.go Outdated Show resolved Hide resolved
beacon-chain/main.go Outdated Show resolved Hide resolved
shayzluf and others added 2 commits April 30, 2020 21:23
Co-authored-by: Victor Farazdagi <simple.square@gmail.com>
Co-authored-by: Victor Farazdagi <simple.square@gmail.com>
@codecov
Copy link

codecov bot commented Apr 30, 2020

Codecov Report

Merging #5694 into master will increase coverage by 32.67%.
The diff coverage is 66.66%.

@@             Coverage Diff             @@
##           master    #5694       +/-   ##
===========================================
+ Coverage   22.26%   54.94%   +32.67%     
===========================================
  Files         180      313      +133     
  Lines       15790    26098    +10308     
===========================================
+ Hits         3516    14339    +10823     
+ Misses      11723     9686     -2037     
- Partials      551     2073     +1522     

beacon-chain/main.go Outdated Show resolved Hide resolved
…in_config

# Conflicts:
#	shared/testutil/spectest.go
beacon-chain/main.go Outdated Show resolved Hide resolved
nisdas
nisdas previously approved these changes May 5, 2020
Copy link
Member

@nisdas nisdas left a comment

Choose a reason for hiding this comment

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

LGTM

beacon-chain/node/node.go Outdated Show resolved Hide resolved
beacon-chain/node/node.go Outdated Show resolved Hide resolved
Copy link
Member

@terencechain terencechain left a comment

Choose a reason for hiding this comment

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

One comment on moving the helper methods out side of node.go

beacon-chain/node/node.go Outdated Show resolved Hide resolved
beacon-chain/node/node.go Outdated Show resolved Hide resolved
@shayzluf shayzluf merged commit 1a27c21 into master May 5, 2020
@delete-merged-branch delete-merged-branch bot deleted the yaml_chain_config branch May 5, 2020 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants