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

KAFKA-5144 renamed and added comments to make it clear what's going on #2948

Closed
wants to merge 1 commit into from

Conversation

mihbor
Copy link
Contributor

@mihbor mihbor commented Apr 30, 2017

The descendingSubsequence is a misnomer. The linked list is actually arranged so that the lowest timestamp is first and larger timestamps are added to the end, therefore renamed to ascendingSubsequence.
The minElem variable was also misnamed. It's actually the current maximum element as it's taken from the end of the list.
Added comment to get() to make it clear it's returning the lowest timestamp.

…going on

The descendingSubsequence is a misnomer. The linked list is actually arranged so that the lowest timestamp is first and larger timestamps are added to the end, therefore renamed to ascendingSubsequence.
The minElem variable was also misnamed. It's actually the current maximum element as it's taken from the end of the list.
Added comment to get() to make it clear it's returning the lowest timestamp.
@asfbot
Copy link

asfbot commented Apr 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/3319/
Test FAILed (JDK 8 and Scala 2.11).

@asfbot
Copy link

asfbot commented Apr 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/3314/
Test PASSed (JDK 7 and Scala 2.10).

@mihbor
Copy link
Contributor Author

mihbor commented Apr 30, 2017

retest this please

@asfbot
Copy link

asfbot commented Apr 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/3320/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link

asfbot commented Apr 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/3315/
Test PASSed (JDK 7 and Scala 2.10).

@asfbot
Copy link

asfbot commented Apr 30, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/3311/
Test PASSed (JDK 8 and Scala 2.12).

Copy link
Member

@mjsax mjsax left a comment

Choose a reason for hiding this comment

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

LGTM.

Call for merging @guozhangwang

Copy link
Member

@mjsax mjsax left a comment

Choose a reason for hiding this comment

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

LGTM.

Call for merging @guozhangwang

Copy link
Contributor

@guozhangwang guozhangwang left a comment

Choose a reason for hiding this comment

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

Merged to trunk.

@asfgit asfgit closed this in bc65c62 May 3, 2017
@guozhangwang
Copy link
Contributor

@mihbor I manually changed your author signature from mihbor <mihbor@users.noreply.github.com> to Michal Borowiecki <mbor81@gmail.com>. Let me know if you prefer the original one, or you could change your local user / email profile accordingly in the future.

@mihbor
Copy link
Contributor Author

mihbor commented May 4, 2017 via email

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