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

Implement XEP-0369/XEP-0405 (MIX): IQ queries #174

Merged
merged 1 commit into from
Jan 20, 2019

Conversation

lnjX
Copy link
Member

@lnjX lnjX commented Jan 3, 2019

This implements all used IQ queries of XEP-0369: Mediated Information
eXchange (MIX) (v0.14.0) and XEP-0405: Mediated Information eXchange (MIX):
Participant Server Requirements (v0.3.0), including unit tests.

Happy reviewing! :D

@lnjX
Copy link
Member Author

lnjX commented Jan 3, 2019

Do you have any idea why some of the travis builds fail (I'm not having that error locally with gcc 8.2)? Didn't I do exactly the same as in the other tests?


edit:
.... of course I didn't do it as in the other tests and used the new ::addRow() instead of ::newRow().

@lnjX lnjX force-pushed the feature/mix-iqs branch from 4d4013f to def5fce Compare January 4, 2019 00:21
@codecov
Copy link

codecov bot commented Jan 4, 2019

Codecov Report

Merging #174 into master will increase coverage by 0.59%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #174      +/-   ##
==========================================
+ Coverage   65.27%   65.87%   +0.59%     
==========================================
  Files         146      149       +3     
  Lines       14591    14846     +255     
==========================================
+ Hits         9525     9780     +255     
  Misses       5066     5066
Impacted Files Coverage Δ
src/base/QXmppMixIq.cpp 100% <100%> (ø)
tests/qxmppmixiq/tst_qxmppmixiq.cpp 100% <100%> (ø)
src/base/QXmppMixIq.h 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a34abc...2c61c4d. Read the comment docs.

@lnjX lnjX force-pushed the feature/mix-iqs branch from def5fce to ee7c3ec Compare January 4, 2019 12:44
@lnjX
Copy link
Member Author

lnjX commented Jan 4, 2019

... now it's really fixed (there was a ( too much)

@lnjX lnjX force-pushed the feature/mix-iqs branch 2 times, most recently from 50f6861 to 90b071a Compare January 4, 2019 13:53
@lnjX
Copy link
Member Author

lnjX commented Jan 4, 2019

Now also added a test for ::isMixIq(), should be ready now.

@lnjX
Copy link
Member Author

lnjX commented Jan 16, 2019

ping! :)
It were nice if you could review & merge the three MIX pull requests soon: It were also easier for me to work on the MixManager then.

child = child.firstChildElement();
}

if (child.isNull() || child.namespaceURI() != ns_mix)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm finding this condition a bit confusing, maybe turn it into a "positive" condition?

if (!child.isNull() && child.namespaceURI() == ns_mix) {
// do something with the child
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done :)

{
QDomElement child = element.firstChildElement();
// determine action type
m_actionType = (QXmppMixIq::Type) MIX_ACTION_TYPES.indexOf(child.tagName());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this blow up if child is null?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it won't. I also added a test for the empty case.

This implements all used IQ queries of XEP-0369: Mediated Information
eXchange (MIX) (v0.14.1) and XEP-0405: Mediated Information eXchange (MIX):
Participant Server Requirements (v0.3.1), including unit tests.
@jlaine jlaine merged commit 95063b1 into qxmpp-project:master Jan 20, 2019
@lnjX lnjX deleted the feature/mix-iqs branch January 20, 2019 20:35
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