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-2316: comment does not match code logic #223

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/java/main/org/apache/zookeeper/server/quorum/Leader.java
Expand Up @@ -724,6 +724,8 @@ synchronized public boolean tryToCommit(Proposal p, long zxid, SocketAddress fol
// concurrent reconfigs are allowed, this can happen.
if (outstandingProposals.containsKey(zxid - 1)) return false;

// in order to be committed, a proposal must be accepted by a quorum
//
Copy link
Contributor

Choose a reason for hiding this comment

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

This blank line can be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hanm I think the blank line may be fine, it provides separation from the idea on the next line so it doesn't read as a single sentence.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would just use a period to do the separation.

// getting a quorum from all necessary configurations
if (!p.hasAllQuorums()) {
return false;
Expand All @@ -737,8 +739,6 @@ synchronized public boolean tryToCommit(Proposal p, long zxid, SocketAddress fol
+ (lastCommitted+1));
}

// in order to be committed, a proposal must be accepted by a quorum

outstandingProposals.remove(zxid);

if (p.request != null) {
Expand Down