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

Zookeeper-3457.Code optimization in QuorumCnxManager #1021

Closed
wants to merge 3 commits into from

Conversation

finefuture
Copy link

more details in the ZOOKEEPER-3457

@maoling
Copy link
Member

maoling commented Jul 23, 2019

Haha, this PR has a traffic collision with PR-1020.

@finefuture
Copy link
Author

@maoling Oh, yes, I just submitted PR for my issue.

Copy link
Contributor

@anmolnar anmolnar left a comment

Choose a reason for hiding this comment

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

+1 Nice catch.
I'm willing to accept your patch, because you removed the old code instead of commenting out, despite that the other came a little bit earlier.

@finefuture
Copy link
Author

I just think useless code should be deleted

@nkalmar
Copy link
Contributor

nkalmar commented Jul 26, 2019

Please assign the jira to yourself if you start working on it, and update to "in progress".
Otherwise multiple people could start working on it, like it happened now.

Edit: since you created the jira, and wrote the solution in the description, I'll also +1 this instead of the other PR.

edit2: I couldn't assign you to the jira, you might need to be added or I think after the first commit you'll automatically get the permission for the jira? Anyhow, you can write to dev@zookeeper.apache.org to be added.

Thanks!

Copy link
Contributor

@nkalmar nkalmar left a comment

Choose a reason for hiding this comment

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

+1

@finefuture
Copy link
Author

@nkalmar It's embarrassing that I can't assign a user

Copy link
Member

@maoling maoling left a comment

Choose a reason for hiding this comment

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

ping @eolivelli

@asfgit asfgit closed this in 83940b1 Jul 27, 2019
@anmolnar
Copy link
Contributor

Committed to master branch. Thanks @finefuture !
I added you to the contributors group in Jira, so now you're able to assing Jiras yourself.

stickyhipp pushed a commit to stickyhipp/zookeeper that referenced this pull request Aug 2, 2019
more details in the [ZOOKEEPER-3457](https://issues.apache.org/jira/browse/ZOOKEEPER-3457)

Author: longqiang <l>

Reviewers: nkalmar@apache.org, andor@apache.org

Closes apache#1021 from finefuture/zookeeper-3457 and squashes the following commits:

3126150 [longqiang] refactor(optimize code):
541547d [longqiang] Merge branch 'master' of github.com:apache/zookeeper into zookeeper-3457
ea78fe8 [longqiang] refactor(optimize code):
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
more details in the [ZOOKEEPER-3457](https://issues.apache.org/jira/browse/ZOOKEEPER-3457)

Author: longqiang <l>

Reviewers: nkalmar@apache.org, andor@apache.org

Closes apache#1021 from finefuture/zookeeper-3457 and squashes the following commits:

3126150 [longqiang] refactor(optimize code):
541547d [longqiang] Merge branch 'master' of github.com:apache/zookeeper into zookeeper-3457
ea78fe8 [longqiang] refactor(optimize code):
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
more details in the [ZOOKEEPER-3457](https://issues.apache.org/jira/browse/ZOOKEEPER-3457)

Author: longqiang <l>

Reviewers: nkalmar@apache.org, andor@apache.org

Closes apache#1021 from finefuture/zookeeper-3457 and squashes the following commits:

3126150 [longqiang] refactor(optimize code):
541547d [longqiang] Merge branch 'master' of github.com:apache/zookeeper into zookeeper-3457
ea78fe8 [longqiang] refactor(optimize code):
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.

4 participants