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

MYST-81 reuse node identity #53

Merged
merged 6 commits into from Dec 6, 2017

Conversation

Projects
None yet
4 participants

@ignasbernotas ignasbernotas self-assigned this Dec 5, 2017

@ignasbernotas ignasbernotas requested review from shroomist, tadovas, interro and Waldz Dec 5, 2017

err := cache.StoreIdentity(&identity)
assert.Nil(t, err)

_, err = os.Stat(file)

This comment has been minimized.

Copy link
@tadovas

tadovas Dec 5, 2017

Member

Purpose of this line? err is ignored anyway.

This comment has been minimized.

Copy link
@ignasbernotas

ignasbernotas Dec 5, 2017

Author Member

It's for a test, i don't need the file info, I just want to check if it exists

This comment has been minimized.

Copy link
@ignasbernotas

ignasbernotas Dec 5, 2017

Author Member

Fixed


import "github.com/mysterium/node/service_discovery/dto"

type identityController struct {

This comment has been minimized.

Copy link
@tadovas

tadovas Dec 5, 2017

Member

Naming. Usually name Controller stands for C in MVC paradigm and it is something facing user, handling its actions and rendering views. Maybe indentityController can be renamed to something like identityManager to avoid confusion with widely adopted terms.

This comment has been minimized.

Copy link
@ignasbernotas

ignasbernotas Dec 5, 2017

Author Member

how about identityHandler?

This comment has been minimized.

Copy link
@ignasbernotas

ignasbernotas Dec 5, 2017

Author Member

renamed to identityHandler

@shroomist
Copy link
Contributor

left a comment

looks good to me

@ignasbernotas

This comment has been minimized.

Copy link
Member Author

commented Dec 6, 2017

Can I merge? @tadovas @Waldz

@Waldz

Waldz approved these changes Dec 6, 2017

@Waldz Waldz merged commit 664983f into master Dec 6, 2017

@Waldz Waldz deleted the feature/MYST-81-reuse-node-identity branch Dec 6, 2017

@Waldz Waldz changed the title Feature/myst 81 reuse node identity MYST-81 reuse node identity Dec 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.