Skip to content

Fix issue with num_results query param #134

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 30, 2023
Merged

Fix issue with num_results query param #134

merged 1 commit into from
Nov 30, 2023
+3 −2

Conversation

@jjl014
Copy link
Contributor

@jjl014 jjl014 commented Nov 30, 2023

Updates:

  • Fix query param for num_results
    • There was a small typo/bug in our constants file that was causing issues when trying to retrieve more than 10 recommendation results at a time (default is 10).
    • It was setting the query param as num_result which is incorrect
  • Update test

@jjl014 jjl014 requested a review from a team November 30, 2023 22:50
stanlp1
stanlp1 approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@stanlp1 stanlp1 left a comment

Choose a reason for hiding this comment

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

LGTM!.

Mudaafi
Mudaafi approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@Mudaafi Mudaafi left a comment

Choose a reason for hiding this comment

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

Tests pass, LGTM!

@jjl014 jjl014 merged commit 24176ad into master Nov 30, 2023
@jjl014 jjl014 deleted the nocsl-android-recs-num-results-fix branch November 30, 2023 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mudaafi Mudaafi Mudaafi approved these changes

@stanlp1 stanlp1 stanlp1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jjl014 @Mudaafi @stanlp1