[Feature] Identity carry over part 6 / Configure Identity from constuctor
#194
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 30 commits
November 24, 2016 15:28
also make sure the local labels map is immutable
… is reflected on the ring
thanodnl
reviewed
Dec 14, 2016
|
|
||
| #### Change to identity-option | ||
|
|
||
| Prior to ringpop NEXT the address was used as the identity of a member. Starting with version NEXT, it's possible to |
There was a problem hiding this comment.
Don't forget to change NEXT here as well when releasing.
thanodnl
reviewed
Dec 14, 2016
|
|
||
| // IdentityResolverFunc is used to specify a function that will called when | ||
| // the Ringpop instance needs to resolve its identity (typically, on | ||
| // AddressResolverFunc is used to specify a function that will called when |
There was a problem hiding this comment.
nit: s/will called/will be called/
thanodnl
approved these changes
Dec 14, 2016
| func Identity(identity string) Option { | ||
| return func(r *Ringpop) error { | ||
| if util.HostportPattern.MatchString(identity) { | ||
| return errors.New("a hostport is not valid as an identity") |
There was a problem hiding this comment.
maybe define an error at the top that is exported and returned here.
There was a problem hiding this comment.
Was thinking the same but all other errors in this file are created inline. I chose to follow the code style (of this file).
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| declare project_root="${0%/*}/.." | |||
| declare ringpop_common_dir="${0%/*}/ringpop-common" | |||
| declare ringpop_common_branch="menno/allow-multi-checksum" | |||
| declare ringpop_common_branch="feature/identity" | |||
There was a problem hiding this comment.
Lets make sure to change this to master before landing.
added 21 commits
December 15, 2016 13:08
moved addressChecksummer to a test file since it’s only used for validation backwards compatibility.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of specifying the identity after bootstrap, rename the existing
IdentityandIdentityResolverFuncoptions toAddressandAddressResolverFuncand add anIdentityoption to specify the identity.Note: this introduces a backwards incompatible change.
CHANGES.mdcontains ago fmtcommand to resolve any issues.