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

identity: createConfig #741

Merged
merged 8 commits into from
Feb 12, 2019
Merged

identity: createConfig #741

merged 8 commits into from
Feb 12, 2019

Conversation

xmxanuel
Copy link
Member

  • create config with new identity

@xmxanuel xmxanuel added the WIP in progress label Feb 11, 2019
@codecov
Copy link

codecov bot commented Feb 11, 2019

Codecov Report

Merging #741 into develop will increase coverage by 0.02%.
The diff coverage is 35.13%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #741      +/-   ##
===========================================
+ Coverage    53.31%   53.34%   +0.02%     
===========================================
  Files          119      119              
  Lines         9110     9208      +98     
===========================================
+ Hits          4857     4912      +55     
- Misses        3678     3717      +39     
- Partials       575      579       +4
Impacted Files Coverage Δ
identity/identity.go 87.69% <ø> (ø) ⬆️
identity/did/key.go 46.15% <0%> (-8.4%) ⬇️
bootstrap/bootstrappers/bootstrapper.go 5.12% <0%> (-0.14%) ⬇️
identity/did/bootstrapper.go 78.04% <100%> (ø) ⬆️
testworld/park.go 77.18% <100%> (ø) ⬆️
identity/did/service.go 42.4% <6.38%> (-12.39%) ⬇️
identity/did/factory.go 45.33% <7.69%> (-7.7%) ⬇️
cmd/common.go 39.56% <71.11%> (+39.56%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b750c5e...10a5e40. Read the comment docs.

@xmxanuel xmxanuel removed the WIP in progress label Feb 12, 2019
Copy link
Contributor

@mikiquantum mikiquantum left a comment

Choose a reason for hiding this comment

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

Some structure comments.

cmd/common.go Outdated
@@ -58,6 +68,110 @@ func addKeys(config config.Configuration, idService identity.Service) error {
return nil
}

func getKeyPairsFromConfig(config config.Configuration) (map[int]did.Key, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this function should be more in the identity package as it will be reused from the API as well?

cmd/common.go Outdated
return keys, nil
}

func addKeysFromConfig(ctx map[string]interface{}, cfg config.Configuration) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

cmd/common.go Outdated
return nil
}

func createIdentity(ctx map[string]interface{}, cfg config.Configuration, configFile *viper.Viper) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@@ -80,7 +80,7 @@ func getAnchorAddress() common.Address {
// ---------------------------------------------------------------------------------------------------------------------
func migrateNewIdentityContracts() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic in this file is only for testing, we should think about removing it to the test bootstapper instead.

@xmxanuel
Copy link
Member Author

ah okay. for the codecov the test needs to be in the same package

@xmxanuel xmxanuel merged commit 2feca77 into centrifuge:develop Feb 12, 2019
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.

None yet

2 participants