-
Notifications
You must be signed in to change notification settings - Fork 643
Add another column to the homepage for "Most Recently Downloaded" #962
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
Comments
I would like to work on this issue but https://crates.io/api/v1/summary api does not return 'Most recently downloaded' information. |
Yep, changing the API for the /api/v1/summary route to return this information is part of the issue actually. I got confused and thought #935 was about that, but it's not. Sorry for the misleading tags! |
I have created a PR for the backend changes requested here. I added some tests. However, oddly, when I run |
Hi @erewok!! Glad to have you here!! The behavior you're seeing with |
Hi @carols10cents. Thanks for the warm welcome! I am impressed with how well-run this project is. At any rate, this morning I cloned the repo anew, built, and ran the tests and the problem seems to have disappeared. I'm not sure why it was ever there in the first place but it must have been user error. Thanks. I'm going to work on fixing the tests I wrote for the |
The backend for this is now done, thank you @erewok !!!! |
@carols10cents As the backend is done, may I work on the frontend part? |
@sabinbajracharya absolutely! |
@carols10cents Can this be closed? |
@someguynamedmatt Indeed! good catch! |
Instructions for fixing this, can be done in 2 separate PRs but they will need to be done in order :)
PR 1: backend
summary
function that returns JSON for the /api/v1/summary route, add a query similar to these queries that selects 10 crates sorted by recent downloads descending nulls last, which is defined by this query that's part of theindex
function (I'm fine with copy-pasting that code, if you feel ambitious and want to refactor to remove the duplication, feel free to give that a try!)CrateBuilder
provides a way to set a crate's recent downloads for testing. The summary route isn't tested very much right now; a bonus would be adding more tests for the other parts of summary as well!PR #2: Frontend
The text was updated successfully, but these errors were encountered: