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

ignore connecting users in qhUsers hash in gRPC implementation #3332

Merged
merged 1 commit into from Feb 8, 2018

Conversation

@bontibon
Copy link
Contributor

commented Feb 8, 2018

Fixes #3299

@bontibon bontibon requested a review from mkrautz Feb 8, 2018

@bontibon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2018

@Johni0702 I believe this patch should fix the crash you were experiencing; it fixed it for me after I replicated the issue.

@bontibon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2018

There are a couple of places I missed where this check should be added. Give me a few hours.

@mkrautz

This comment has been minimized.

Copy link
Member

commented Feb 8, 2018

Squash typo fix into previous commit?

@bontibon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2018

@mkrautz Done

@mkrautz
mkrautz approved these changes Feb 8, 2018
Copy link
Member

left a comment

Seems plausible. Letting you go wild since you practically "own" the GRPC code. :)

@bontibon bontibon merged commit 765f780 into mumble-voip:master Feb 8, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@mkrautz

This comment has been minimized.

Copy link
Member

commented Feb 8, 2018

BTW, please let Travis CI and AppVeyor finish sucessfully before merging, unless it's very urgent.

Also, we tend to use the following template for pull requests:

Merge PR #<PRNum>: <PRTitle>

<OptionalPRBody>

The title is more descriptive than a username and branch, especially if you're grouping commits together in a "feature PR". Then the PR title and message (if present) is used to describe the feature as a whole, and any information that belongs there. This allows the individual commit titles/messages to only concern themselves with their own content.

Typically, the OptionalPRBody is the text in the first post of the PR.
If it's a single commit PR, or a PR body doesn't make sense, feel free to leave it out.

No worries though. I hadn't told you. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.