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

chore: sort categories in recommended app service response #4498

Merged
merged 1 commit into from
May 19, 2024

Conversation

BenjaminX
Copy link
Contributor

No description provided.

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. 🐍 python Pull requests that update Python code labels May 18, 2024
Copy link
Collaborator

@Yeuoly Yeuoly left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 19, 2024
@loganclark360
Copy link

loganclark360 commented May 19, 2024 via email

@@ -95,7 +95,7 @@ def _fetch_recommended_apps_from_db(cls, language: str) -> dict:

categories.add(recommended_app.category) # add category to categories

return {'recommended_apps': recommended_apps_result, 'categories': list(categories)}
return {'recommended_apps': recommended_apps_result, 'categories': sorted(list(categories))}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return {'recommended_apps': recommended_apps_result, 'categories': sorted(list(categories))}
return {'recommended_apps': recommended_apps_result, 'categories': sorted(categories)}

Simply sort the set, and the returned value is in list type. eg.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

list(categories) duplicate removal, and sorted by it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. SGTM.

@takatost takatost merged commit 46bd53a into langgenius:main May 19, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer 🐍 python Pull requests that update Python code size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants