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

Add roomDestroyed to MUC UserStatusListener #34

Closed
wants to merge 0 commits into from
Closed

Add roomDestroyed to MUC UserStatusListener #34

wants to merge 0 commits into from

Conversation

dasfuu
Copy link
Contributor

@dasfuu dasfuu commented Mar 11, 2015

I could not test it yet, but it is pretty simple and looks ok to me.

@@ -65,5 +65,8 @@ public void adminGranted() {

public void adminRevoked() {
}

public void roomDestroyed(Jid alternateJID, String reason) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is the parmater name alternateJID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as here:
MultiUserChat.destroy(String reason, String alternateJID)

Copy link
Member

Choose a reason for hiding this comment

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

Ahh, makes sense then. And I see it's javadoc'ed at the listener, so it's fine.

@Flowdalic
Copy link
Member

Thanks for you contribution. Please

  • squash the commits into one and update this PR
  • mention the issue key SMACK-619 somewhere in the body of the commit message

@dasfuu
Copy link
Contributor Author

dasfuu commented Mar 11, 2015

Squashed the commits

@Flowdalic
Copy link
Member

I've just pushed 516e397

Could you rebase you commit on it, so that we can get rid of the jid parsing code?

@Flowdalic
Copy link
Member

Also please change

public void roomDestroyed(Jid alternateJID, String reason);

to

public void roomDestroyed(MultiUserChat alternativeMuc, String reason);

@dasfuu
Copy link
Contributor Author

dasfuu commented Mar 13, 2015

I will do it when I get home later.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 35.56% when pulling 08a1eed on meisterfuu:master into e126469 on igniterealtime:master.

@dasfuu dasfuu closed this Mar 16, 2015
@dasfuu
Copy link
Contributor Author

dasfuu commented Mar 16, 2015

#36

@Flowdalic
Copy link
Member

FYI, no need to close a PR. You could simply update the existing one by force-pushing to its source branch.

@dasfuu
Copy link
Contributor Author

dasfuu commented Mar 16, 2015

I messed with the fork master and wanted a clean clone from the smack master without a "Merge commit". I rarely do other things than Push/Pull with git, still learning :)

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