Add more documentation

src/git.rs

@@ -73,6 +73,10 @@ pub fn add_crate(app: &App, krate: &Crate) -> CargoResult<()> {
     })
 }
 
+/// Yanks or unyanks a crate version. This requires finding the index
+/// file, deserlialise the crate from JSON, change the yank boolean to
+/// `true` or `false`, write all the lines back out, and commit and
+/// push the changes.
 pub fn yank(app: &App, krate: &str, version: &semver::Version, yanked: bool) -> CargoResult<()> {
     let repo = app.git_repo.lock().unwrap();
     let repo_path = repo.workdir().unwrap();
@@ -112,6 +116,22 @@ pub fn yank(app: &App, krate: &str, version: &semver::Version, yanked: bool) ->
     })
 }
 
+/// Commits and pushes to the crates.io index.
+///
+/// There are currently 2 instances of the crates.io backend running
+/// on Heroku, and they race against each other e.g. if 2 pushes occur,
+/// then one will succeed while the other will need to be rebased before
+/// being pushed.
+///
+/// A maximum of 20 attempts to commit and push to the index currently
+/// accounts for the amount of traffic publishing crates, though this may
+/// have to be changed in the future.
+///
+/// Notes:
+/// Currently, this function is called on the HTTP thread and is blocking.
+/// Spawning a separate thread for this function means that the request
+/// can return without waiting for completion, and other methods of
+/// notifying upon completion or error can be used.
 fn commit_and_push<F>(repo: &git2::Repository, mut f: F) -> CargoResult<()>
 where
     F: FnMut() -> CargoResult<(String, PathBuf)>,

src/krate.rs

@@ -785,6 +785,26 @@ impl Model for Crate {
 }
 
 /// Handles the `GET /crates` route.
+/// Returns a list of crates. Called in a variety of scenarios in the
+/// front end, including:
+/// - Alphabetical listing of crates
+/// - List of crates under a specific owner
+/// - Listing a user's followed crates
+///
+/// Notes:
+/// The different use cases this function covers is handled through passing
+/// in parameters in the GET request.
+///
+/// We would like to stop adding functionality in here. It was built like
+/// this to keep the number of database queries low, though given Rust's
+/// low performance overhead, this is a soft goal to have, and can afford
+/// more database transactions if it aids understandability.
+///
+/// All of the edge cases for this function are not currently covered
+/// in testing, and if they fail, it is difficult to determine what
+/// caused the break. In the future, we should look at splitting this
+/// function out to cover the different use cases, and create unit tests
+/// for them.
 pub fn index(req: &mut Request) -> CargoResult<Response> {
     use diesel::expression::{AsExpression, DayAndMonthIntervalDsl};
     use diesel::types::{Bool, BigInt, Nullable};
@@ -913,6 +933,8 @@ pub fn index(req: &mut Request) -> CargoResult<Response> {
         ));
     }
 
+    // The database query returns a tuple within a tuple , with the root
+    // tuple containing 3 items.
     let data = query
         .paginate(limit, offset)
         .load::<((Crate, bool, Option<i64>), i64)>(&*conn)?;
@@ -1100,6 +1122,12 @@ pub fn show(req: &mut Request) -> CargoResult<Response> {
 }
 
 /// Handles the `PUT /crates/new` route.
+/// Used by `cargo publish` to publish a new crate or to publish a new version of an
+/// existing crate.
+///
+/// Currently blocks the HTTP thread, perhaps some function calls can spawn new
+/// threads and return completion or error through other methods  a `cargo publish
+/// --status` command, via crates.io's front end, or email.
 pub fn new(req: &mut Request) -> CargoResult<Response> {
     let app = req.app().clone();
     let (new_crate, user) = parse_new_headers(req)?;
@@ -1126,6 +1154,8 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
     let categories: Vec<_> = categories.iter().map(|k| &**k).collect();
 
     let conn = req.db_conn()?;
+    // Create a transaction on the database, if there are no errors,
+    // commit the transactions to record a new or updated crate.
     conn.transaction(|| {
         // Persist the new crate, if it doesn't already exist
         let persist = NewCrate {
@@ -1236,6 +1266,11 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
     })
 }
 
+/// Used by the `krate::new` function.
+///
+/// This function parses the JSON headers to interpret the data and validates
+/// the data during and after the parsing. Returns crate metadata and user
+/// information.
 fn parse_new_headers(req: &mut Request) -> CargoResult<(upload::NewCrate, User)> {
     // Read the json upload request
     let amt = read_le_u32(req.body())? as u64;

src/lib.rs

@@ -148,11 +148,14 @@ pub fn middleware(app: Arc<App>) -> MiddlewareBuilder {
         C(version::downloads),
     );
     api_router.get("/crates/:crate_id/:version/authors", C(version::authors));
+    // Used to generate download graphs
     api_router.get("/crates/:crate_id/downloads", C(krate::downloads));
     api_router.get("/crates/:crate_id/versions", C(krate::versions));
     api_router.put("/crates/:crate_id/follow", C(krate::follow));
     api_router.delete("/crates/:crate_id/follow", C(krate::unfollow));
     api_router.get("/crates/:crate_id/following", C(krate::following));
+    // This endpoint may now be redundant, check frontend to see if it is
+    // being used
     api_router.get("/crates/:crate_id/owners", C(krate::owners));
     api_router.get("/crates/:crate_id/owner_team", C(krate::owner_team));
     api_router.get("/crates/:crate_id/owner_user", C(krate::owner_user));

src/upload.rs

@@ -1,3 +1,7 @@
+//! This module handles the expected information a crate should have
+//! and manages the serialising and deserialising of this information
+//! to and from structs. The serlializing is only utilised in
+//! integration tests.
 use std::collections::HashMap;
 use std::ops::Deref;
 

src/version.rs

@@ -461,6 +461,14 @@ pub fn authors(req: &mut Request) -> CargoResult<Response> {
 }
 
 /// Handles the `DELETE /crates/:crate_id/:version/yank` route.
+/// This does not delete a crate version, it makes the crate
+/// version accessible only to crates that already have a
+/// `Cargo.lock` containing this version.
+///
+/// Notes:
+/// Crate deletion is not implemented to avoid breaking builds,
+/// and the goal of yanking a crate is to prevent crates
+/// beginning to depend on the yanked crate version.
 pub fn yank(req: &mut Request) -> CargoResult<Response> {
     modify_yank(req, true)
 }
@@ -470,6 +478,7 @@ pub fn unyank(req: &mut Request) -> CargoResult<Response> {
     modify_yank(req, false)
 }
 
+/// Changes `yanked` flag on a crate version record
 fn modify_yank(req: &mut Request, yanked: bool) -> CargoResult<Response> {
     let (version, krate) = version_and_crate(req)?;
     let user = req.user()?;