Skip to content

XEP-0234: Use <hash-used/> to signify the hash function being used, instead of an (invalid) empty <hash/>#745

Merged
horazont merged 3 commits intoxsf:masterfrom
Ri0n:patch-1
Jun 19, 2019
Merged

XEP-0234: Use <hash-used/> to signify the hash function being used, instead of an (invalid) empty <hash/>#745
horazont merged 3 commits intoxsf:masterfrom
Ri0n:patch-1

Conversation

@Ri0n
Copy link
Contributor

@Ri0n Ri0n commented Jan 20, 2019

Most likely smeone just forgot to update the exampe after introducing hash-used in this xep.

It seems description of example 16 exactly describes <hash-used> case. Moreover empty <hash> is likely invaid according to xep-0300.
@CLAassistant
Copy link

CLAassistant commented Jan 20, 2019

CLA assistant check
All committers have signed the CLA.

@horazont
Copy link
Contributor

This is inconsistent. The text above the example explicitly talks about an empty <hash/> element. This, of course, is in conflict with what XEP-0300 says. So please update the PR so that it also fixes the text above the example.

@horazont horazont self-assigned this Jan 21, 2019
@horazont horazont added the Needs Author The XEP is experimental and the PR was not made by the author. The author needs to acknowledge it. label Jan 21, 2019
@horazont
Copy link
Contributor

@stpeter @legastero Please review.

This changes the effective Schema and might require a namespace bump to fix, even though the previous text was (I think) invalid use of XEP-0300.

@horazont horazont changed the title xep-234: Example 16 should use hash-used XEP-0234: Example 16 should use hash-used Jan 21, 2019
@horazont horazont changed the title XEP-0234: Example 16 should use hash-used XEP-0234: Use <hash-used/> to signify the hash function being used, instead of an (invalid) empty <hash/> Jan 21, 2019
@horazont
Copy link
Contributor

@Ri0n As you can see, I notified the authors for feedback. This may take a few days.

@Ri0n
Copy link
Contributor Author

Ri0n commented Jan 21, 2019

Thanks

@stpeter
Copy link
Member

stpeter commented Jan 22, 2019

Hmm, this does seem to be an error in the text and examples, and would also necessitate a change to the schema (which we should include in this PR). As to whether this requires a namespace bump, I leave that to the Council.

@horazont horazont added Needs Council The affected XEP has the Council as Approving Body and it needs to decide on the change. Needs Changes and removed Needs Author The XEP is experimental and the PR was not made by the author. The author needs to acknowledge it. labels Jan 23, 2019
@horazont
Copy link
Contributor

@stpeter Thanks! I’ll try to squeeze this into today’s Council meeting.

@Ri0n Could you make the changes to the schema?

@Ri0n
Copy link
Contributor Author

Ri0n commented Jan 23, 2019

Added to schema. Not sure if it's correct.
By the way the XEP also notices hash-used can be used in . I believe it's somewhat wrong. Checksum should always provide hash value.

@dwd
Copy link
Contributor

dwd commented May 21, 2019

(Passed Council, 2019-02-20)

@horazont horazont added Needs Version Block The change requires a version block, and this is to be done by Editors at merge time. Ready To Merge No acknowledgements of other parties are needed anymore. There may be changes to do at merge time. and removed Needs Changes Needs Council The affected XEP has the Council as Approving Body and it needs to decide on the change. labels Jun 19, 2019
@horazont horazont merged commit 935fed1 into xsf:master Jun 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Version Block The change requires a version block, and this is to be done by Editors at merge time. Ready To Merge No acknowledgements of other parties are needed anymore. There may be changes to do at merge time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants