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

FIX sort order of view entries with compound keys in mock mode #53

Closed
wants to merge 1 commit into from
Closed

FIX sort order of view entries with compound keys in mock mode #53

wants to merge 1 commit into from

Conversation

snrbrnjna
Copy link
Contributor

The way keys are compared in the Bucket Mock results in a wrong sort order for compound keys with integers.

In Mock mode I get view results like that:

[1,1]
[1,10]
[1,100]
[1,2]
[1,3]

@snrbrnjna
Copy link
Contributor Author

Hi @brett19,
I'm currently working with couchnode on a daily basis and due to this error in the Bucket Mock I have to run the tests twice, trice, ... to complete them in green state.
Would be great if you have a short look at my pull req, as it fixes this odd behaviour.
thanks,
mat

@brett19
Copy link
Member

brett19 commented Feb 2, 2016

Thanks for the pull request!! To ensure quality review, Couchbase employs a code review system based on Gerrit to manage the workflow of changes in addition to tracking our contributor agreements.

To get this change in and collaborate in code review, please register on Gerrit and accept our CLA. More detailed instructions are available here: http://developer.couchbase.com/open-source-projects#how-to-contribute-code.

Note: Please contact us if you have any issues registering with Gerrit! If you are not registered after 7 days, the Pull Request will automatically be closed.

@brett19 brett19 closed this Feb 2, 2016
@brett19 brett19 reopened this Feb 2, 2016
@brett19
Copy link
Member

brett19 commented Feb 2, 2016

Sorry for the unintentional close/reopen here. The bot is a bit overzealous sometimes.

@snrbrnjna
Copy link
Contributor Author

registering on https://review.couchbase.org was no problem but now what?
sorry, but i need more instructions for this! is this pull request on github useless? or how do i find it in the review system?

my gerrit usernam is snrbrnjna

@brett19 brett19 closed this Feb 4, 2016
@snrbrnjna
Copy link
Contributor Author

@brett19 ??

@ingenthr
Copy link

ingenthr commented Feb 4, 2016

Sorry for the odd replies @snrbrnjna – we've written a bot to simplify all of this but it looks like there are a few cases we've not anticipated. I'm sure @brett19 will fix this one shortly.

@ingenthr ingenthr reopened this Feb 4, 2016
@brett19 brett19 closed this Feb 4, 2016
@ingenthr ingenthr reopened this Feb 4, 2016
@brett19 brett19 closed this Feb 4, 2016
@brett19
Copy link
Member

brett19 commented Feb 4, 2016

Holy moley. I sincerely apologize for the complications here @snrbrnjna. It looks like you've registered at the review site, but still need to login and accept the CLA (which can be done here: http://review.couchbase.org/#/settings/new-agreement).
P.S. I've fixed the bot as well.
Cheers, Brett

@brett19 brett19 reopened this Feb 4, 2016
@snrbrnjna
Copy link
Contributor Author

the review site states Agreement already submitted.

i don't find the pull request in the gerrit system.

@brett19
Copy link
Member

brett19 commented Feb 5, 2016

Thanks for the pull request!! To ensure quality review, Couchbase employs a code review system based on Gerrit to manage the workflow of changes in addition to tracking our contributor agreements.

To get this change in and collaborate in code review, please register on Gerrit and accept our CLA. More detailed instructions are available here: http://developer.couchbase.com/open-source-projects#how-to-contribute-code.

Note: Please contact us if you have any issues registering with Gerrit! If you are not registered after 7 days, the Pull Request will automatically be closed.

::SDKBOT/PR:no_cla

@brett19
Copy link
Member

brett19 commented Feb 9, 2016

Hey @snrbrnjna,

The typically means that the email you have on your PR (GitHub) doesn't match any of the emails you have on your Gerrit account. If you update gerrit with the correct email, it should pick it up and transfer it over!

Cheers, Brett

@snrbrnjna
Copy link
Contributor Author

sorry, i fixed the emails, but my PR isn't listed in gerrit.

@brett19
Copy link
Member

brett19 commented Feb 11, 2016

Transferred commit (05c5102) to Couchbase Review site:
http://review.couchbase.org/#/c/59790

::SDKBOT/PR:created

@snrbrnjna
Copy link
Contributor Author

ahhh

@snrbrnjna
Copy link
Contributor Author

hey @brett19,
did you manage to review the transferred commit?
cheers,
mat

@brett19
Copy link
Member

brett19 commented Mar 1, 2016

This has been merged through Gerrit. Thanks for your contribution!

@brett19 brett19 closed this Mar 1, 2016
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