Define top(number) helper method #50
Conversation
|
@robertomiranda After thinking about this a bit more, I think I'd rather see explicit |
|
@czarneckid make more sense 👍 |
robertomiranda
force-pushed
the
top-x
branch
2 times, most recently
from
9f2988f
to
689a887
Compare
|
@czarneckid done 😁 |
| # @param options [Hash] Options to be used when retrieving the data from the leaderboard. | ||
| # | ||
| # @return number from the leaderboard that fall within the given rank range. | ||
| def top(number,options = {}) |
There was a problem hiding this comment.
Could you add a space here between the arguments?
|
Looks good. Could you please round out the specs to cover the other leaderboard types (reverse option, Tie and Competition)? |
|
And if you wouldn't mind squashing the commits after you're all done, that'd be great. Thanks in advance. |
robertomiranda
changed the title
| @@ -197,6 +197,69 @@ | |||
| end | |||
| end | |||
|
|
|||
| it 'should allow you to retrieve a given set of members from the leaderboard in a rank range' do | |||
There was a problem hiding this comment.
@czarneckid I realized that in Tie Ranking type were missing the test case for members_from_rank_range. also do you think that this is ready for the commits squash?
There was a problem hiding this comment.
I think it's ready for the commits squash. I think the only other spec to add would be for the Competition Ranking type.
Add `top(number)` method as a shorthand of `members_from_rank_range(1,number) Add `top_in(leaderboard_name, number)` method as a shorthan of `members_from_rank_range_in(leaderboard_name, 1, number)` Testing top methods for rever leadearboard type Testing Againts to Tie Ranking Leaderboard Testing Competition Ranking Leaderboard
robertomiranda
force-pushed
the
top-x
branch
from
9cf2d88
to
af7d296
Compare
|
@czarneckid done! |
|
I'll look at this more in-depth over the next few days, but otherwise things look great. I may add some more specs that add more tie scores to the tie and competition ranking leaderboard types. |
|
@czarneckid any other suggestion? |
|
No. I've got some time over the next week to get this integrated. Sorry it's been awhile. End of year, holidays and getting back into the swing of things here at work. |
|
sure np 👍, I was just wondering 😄 |
I just wanted to add a shorthand for get top_10 of a specific leaderboard, but this cover a general way:
I'll add the test cases and proper document if this method fit with the library