Split out conference type (text/av) from identifier.#1016
Merged
Conversation
|
Looks good, except that now the field is called 'identifier' while elsewhere
in the code it's called a 'uid'. I think it would be clearer if the field were
also called 'uid'.
I'd also suggest renaming 'uid' to 'id', to avoid the potential for
misunderstanding it as "user ID" and to fit the spec, which uses this
terminology ('id' for the group public key and 'identifier' for the 33-byte
type+id).
|
Member
Author
|
@zugz would you suggest renaming the public API function as well? |
Member
Author
|
Anyway, I've renamed identifier to uid. |
b265edb to
bcb24ff
Compare
8d41014 to
1d3e5fe
Compare
Member
Author
|
@zugz done. |
1d3e5fe to
a4ef748
Compare
Member
Author
|
Should we revert "id" back to "identifier"? |
a4ef748 to
150d5dd
Compare
|
Should we revert "id" back to "identifier"?
I'd say it's better to have new terminology for the new concept, to
minimise confusion between versions. It also means we don't have to
change the spec.
<img class="emoji" title=":lgtm_strong:" alt=":lgtm_strong:" align="absmiddle" src="https://reviewable.io/lgtm_strong.png" height="20" width="61"/>
|
zugz
approved these changes
Aug 8, 2018
ea58e0e to
08999a6
Compare
08999a6 to
678720d
Compare
This file contains hidden or 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
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.
This change is