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

Added publish options for device and boundle nodes #1117

Merged
merged 1 commit into from
Feb 10, 2019

Conversation

mimi89999
Copy link
Contributor

No description provided.

@chrisballinger
Copy link
Collaborator

Looks like there are some failing tests:

OMEMOElementTests

  testBundleParsing, �[31m(([iq XMLStringWithOptions:NSXMLNodePrettyPrint]) equal to ([expectedXML XMLStringWithOptions:NSXMLNodePrettyPrint])) failed: ("<iq type="set" id="announce2">�[0m

@mimi89999
Copy link
Contributor Author

I will update the tests.

I am quite worried about code duplication. In XEP-0060 there is a method for publishing to a node with access model, but it is not quite usable here as it already adds the stanza to the queue and returns only the uuid.

@mimi89999 mimi89999 force-pushed the master branch 2 times, most recently from c118b27 to 2388b17 Compare February 5, 2019 11:52
@mimi89999
Copy link
Contributor Author

I updated the test. Could you please review this pull request?

Extensions/OMEMO/XMPPIQ+OMEMO.m Outdated Show resolved Hide resolved
Extensions/OMEMO/XMPPIQ+OMEMO.m Outdated Show resolved Hide resolved
Extensions/OMEMO/XMPPIQ+OMEMO.m Outdated Show resolved Hide resolved
Extensions/OMEMO/XMPPIQ+OMEMO.m Outdated Show resolved Hide resolved
@chrisballinger
Copy link
Collaborator

Thanks for the bump, looks good! I haven't tested it on my end yet but I think it's good to merge. If you want to make changes regarding my comments above I'll hold off on merging until I hear back from you, but it's not a dealbreaker either way.

@mimi89999
Copy link
Contributor Author

I changed the names to lowercase.

@chrisballinger chrisballinger merged commit 19e8d02 into robbiehanson:master Feb 10, 2019
@mimi89999
Copy link
Contributor Author

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants