Skip to content

Return crate id from match_version #569

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

Closed
jyn514 opened this issue Jan 23, 2020 · 3 comments · Fixed by #606
Closed

Return crate id from match_version #569

jyn514 opened this issue Jan 23, 2020 · 3 comments · Fixed by #606
Labels
E-easy Effort: Should be easy to implement and would make a good first PR mentor This has instructions for getting started

Comments

@jyn514
Copy link
Member

jyn514 commented Jan 23, 2020

If I look at where match_version is used, it's always followed by a query on the database fetching the rustdoc_status.
- here we fetch rustdoc_status: https://github.com/rust-lang/docs.rs/blob/master/src/web/releases.rs#L486
- here we fetch rustdoc_status and target_name: https://github.com/rust-lang/docs.rs/blob/master/src/web/rustdoc.rs#L160

Maybe we can change match_version to expose some additional information that makes the next queries easier or, even better, unnecessary?
A simple tweak would be to return the id of the release, making the next query easier.

Originally posted by @koenaad in #565 (comment)

@jyn514
Copy link
Member Author

jyn514 commented Jan 23, 2020

Mentoring instructions:

  • Change the Exact and Semver variants on MatchVersion to have a (String, id) tuple, not just a String.
  • Change match_version to query for id as well as version.
  • Change the usages of match version (linked above) to use that id instead of crates inner join releases ...

@jyn514 jyn514 added E-easy Effort: Should be easy to implement and would make a good first PR mentor This has instructions for getting started labels Jan 23, 2020
@zeegomo
Copy link
Contributor

zeegomo commented Feb 6, 2020

Hi, I'd like to work on this if it's still unclaimed!

@jyn514
Copy link
Member Author

jyn514 commented Feb 6, 2020

@zeegomo go for it! If you have any questions, feel free to reach out here or on Discord. There's also lots of information in the README.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Effort: Should be easy to implement and would make a good first PR mentor This has instructions for getting started
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants