-
Notifications
You must be signed in to change notification settings - Fork 123
XEP-0353/JMI: Fix specification and xml schema inconsistencies #1436
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
base: master
Are you sure you want to change the base?
Conversation
94d02fa
to
63c1711
Compare
Ping @tmolitor-stud-tu :) |
63c1711
to
1053af4
Compare
Ping @fippo @stpeter @tmolitor-stud-tu and @iNPUTmice :) |
Unfortunately, afaik, the SHOULD --> MUST requires a namespace bump and we should avoid this. |
@tmolitor-stud-tu I see. :/ However, the XML schema already had If so, then a JMI element without the However, this is maybe a bit pedantic and I can totally understand if you say this leads to confusion and making the MUST from the schema a SHOULD would be better than the current situation. :) |
What do the existing implementations generate and expect? |
xmpp-parses/Rust, slixmpp, movim, dino all ignore the reason element and also don't generate it. I didn't have a look into the code of the other implementations, but it seems like making it optional is the way to go then. I'll adjust the PR. |
Instead import <reason/> element from Jingle namespace, so the XML schema matches with the examples.
Currently most implementations don't use it, so the best way seems to be making it optional in the schema, too. The text already says SHOULD.
1053af4
to
e589484
Compare
Sorry that this took so long, did the adjustments now. :) |
Currently the schema says some elements MUST contain a '<reason/>' from
urn:xmpp:jingle-message:0
and the examples and specification text said that some elements SHOULD contain a<reason/>
element fromurn:xmpp:jingle:1
.This does:
urn:xmpp:jingle:1
in both XML schema and examples