Merge pull request #1444 from input-output-hk/djo/1439/missing_warn_m…

…issing_docs_in_client_cli Warn missing docs directive missing in Mithril Client Cli
input-output-hk · Jan 18, 2024 · 0573284 · 0573284
2 parents 27ded3e + 28eaed8
commit 0573284
Show file tree

Hide file tree

Showing 17 changed files with 42 additions and 43 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -648,7 +648,9 @@ jobs:
 
       - name: Generate cargo doc
         run: |
-          cargo doc --no-deps -p mithril-stm -p mithril-common -p mithril-aggregator \
+          # Force `--lib` to avoid a collision between the client lib and the client cli binary who share
+          # the same name (we only want to document those anyway)
+          cargo doc --no-deps --lib -p mithril-stm -p mithril-common -p mithril-aggregator \
                     -p mithril-signer -p mithril-client -p mithril-client-cli \
                     --all-features --message-format=json \
                     | clippy-sarif | tee rust-cargo-doc-results.sarif | sarif-fmt

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/mithril-client-cli/Cargo.toml b/mithril-client-cli/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "mithril-client-cli"
-version = "0.5.15"
+version = "0.5.16"
 description = "A Mithril Client"
 authors = { workspace = true }
 edition = { workspace = true }

diff --git a/mithril-client-cli/src/commands/mithril_stake_distribution/download.rs b/mithril-client-cli/src/commands/mithril_stake_distribution/download.rs
@@ -8,8 +8,8 @@ use std::{
     sync::Arc,
 };
 
+use crate::{configuration::ConfigParameters, utils::ExpanderUtils};
 use mithril_client::{ClientBuilder, MessageBuilder};
-use mithril_client_cli::{configuration::ConfigParameters, utils::ExpanderUtils};
 use mithril_common::StdResult;
 
 /// Download and verify a Mithril Stake Distribution information. If the

diff --git a/mithril-client-cli/src/commands/mithril_stake_distribution/list.rs b/mithril-client-cli/src/commands/mithril_stake_distribution/list.rs
@@ -4,8 +4,8 @@ use config::{builder::DefaultState, ConfigBuilder};
 use slog_scope::logger;
 use std::{collections::HashMap, sync::Arc};
 
+use crate::configuration::ConfigParameters;
 use mithril_client::ClientBuilder;
-use mithril_client_cli::configuration::ConfigParameters;
 use mithril_common::{test_utils::fake_keys, StdResult};
 
 /// Mithril stake distribution LIST command

diff --git a/mithril-client-cli/src/commands/snapshot/download.rs b/mithril-client-cli/src/commands/snapshot/download.rs
@@ -10,16 +10,16 @@ use std::{
     sync::Arc,
 };
 
-use mithril_client::{
-    common::ProtocolMessage, Client, ClientBuilder, MessageBuilder, MithrilCertificate, Snapshot,
-};
-use mithril_client_cli::{
+use crate::{
     configuration::ConfigParameters,
     utils::{
         ExpanderUtils, IndicatifFeedbackReceiver, ProgressOutputType, ProgressPrinter,
         SnapshotUnpacker, SnapshotUtils,
     },
 };
+use mithril_client::{
+    common::ProtocolMessage, Client, ClientBuilder, MessageBuilder, MithrilCertificate, Snapshot,
+};
 use mithril_common::StdResult;
 
 /// Clap command to download the snapshot and verify the certificate.
@@ -59,7 +59,7 @@ impl SnapshotDownloadCommand {
         let progress_output_type = if self.json {
             ProgressOutputType::JsonReporter
         } else {
-            ProgressOutputType::TTY
+            ProgressOutputType::Tty
         };
         let progress_printer = ProgressPrinter::new(progress_output_type, 5);
         let client = ClientBuilder::aggregator(

diff --git a/mithril-client-cli/src/commands/snapshot/list.rs b/mithril-client-cli/src/commands/snapshot/list.rs
@@ -4,8 +4,8 @@ use config::{builder::DefaultState, ConfigBuilder};
 use slog_scope::logger;
 use std::{collections::HashMap, sync::Arc};
 
+use crate::configuration::ConfigParameters;
 use mithril_client::ClientBuilder;
-use mithril_client_cli::configuration::ConfigParameters;
 use mithril_common::{test_utils::fake_keys, StdResult};
 
 /// Clap command to list existing snapshots

diff --git a/mithril-client-cli/src/commands/snapshot/show.rs b/mithril-client-cli/src/commands/snapshot/show.rs
@@ -5,8 +5,8 @@ use config::{builder::DefaultState, ConfigBuilder};
 use slog_scope::logger;
 use std::{collections::HashMap, sync::Arc};
 
+use crate::{configuration::ConfigParameters, utils::ExpanderUtils};
 use mithril_client::ClientBuilder;
-use mithril_client_cli::{configuration::ConfigParameters, utils::ExpanderUtils};
 use mithril_common::{test_utils::fake_keys, StdResult};
 
 /// Clap command to show a given snapshot

diff --git a/mithril-client-cli/src/configuration.rs b/mithril-client-cli/src/configuration.rs
@@ -24,6 +24,7 @@ impl ConfigParameters {
     }
 
     /// Useful constructor for testing
+    #[cfg(test)]
     pub fn build(parameters: &[(&str, &str)]) -> Self {
         let parameters = parameters
             .iter()
@@ -34,6 +35,7 @@ impl ConfigParameters {
     }
 
     /// Add or replace a parameter in the holder
+    #[cfg(test)]
     pub fn add_parameter(&mut self, name: &str, value: &str) -> &mut Self {
         let _ = self.parameters.insert(name.to_string(), value.to_string());
 

diff --git a/mithril-client-cli/src/lib.rs b/mithril-client-cli/src/lib.rs
@@ -1,2 +1,12 @@
-pub mod configuration;
-pub mod utils;
+#![warn(missing_docs)]
+
+//! A command line interface that uses the [Mithril Client Library](https://mithril.network/doc/manual/developer-docs/nodes/mithril-client-library)
+//! to manipulate Mithril certified types from a Mithril Aggregator:
+//! * Snapshots: List, Show, download and verify
+//! * Mithril Stake Distribution: List, download and verify
+//
+//! You can find more information on how it works reading the [documentation website](https://mithril.network/doc/mithril/mithril-network/client).
+
+pub mod commands;
+mod configuration;
+mod utils;
diff --git a/mithril-client-cli/src/main.rs b/mithril-client-cli/src/main.rs
@@ -1,7 +1,5 @@
 #![doc = include_str!("../README.md")]
 
-mod commands;
-
 use anyhow::Context;
 use clap::{Parser, Subcommand};
 use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value, ValueKind};
@@ -15,7 +13,7 @@ use std::{fs::File, path::PathBuf};
 
 use mithril_common::StdResult;
 
-use commands::{
+use mithril_client_cli::commands::{
     mithril_stake_distribution::MithrilStakeDistributionCommands, snapshot::SnapshotCommands,
 };
 

diff --git a/mithril-client-cli/src/utils/expander.rs b/mithril-client-cli/src/utils/expander.rs
@@ -2,6 +2,7 @@ use anyhow::anyhow;
 use futures::Future;
 use mithril_common::StdResult;
 
+/// Utilities to expand aliases into their associated ids.
 pub struct ExpanderUtils;
 
 impl ExpanderUtils {

diff --git a/mithril-client-cli/src/utils/feedback_receiver.rs b/mithril-client-cli/src/utils/feedback_receiver.rs
@@ -35,7 +35,7 @@ impl FeedbackReceiver for IndicatifFeedbackReceiver {
                 download_id: _,
                 size,
             } => {
-                let pb = if self.output_type == ProgressOutputType::TTY {
+                let pb = if self.output_type == ProgressOutputType::Tty {
                     ProgressBar::new(size)
                 } else {
                     ProgressBar::with_draw_target(Some(size), ProgressDrawTarget::hidden())
@@ -68,7 +68,7 @@ impl FeedbackReceiver for IndicatifFeedbackReceiver {
             MithrilEvent::CertificateChainValidationStarted {
                 certificate_chain_validation_id: _,
             } => {
-                let pb = if self.output_type == ProgressOutputType::TTY {
+                let pb = if self.output_type == ProgressOutputType::Tty {
                     ProgressBar::new_spinner()
                 } else {
                     ProgressBar::hidden()

diff --git a/mithril-client-cli/src/utils/mod.rs b/mithril-client-cli/src/utils/mod.rs
@@ -1,5 +1,5 @@
 //! Utilities module
-//! This module contains tools needed mostly in services layers.
+//! This module contains tools needed for the commands layer.
 
 mod expander;
 mod feedback_receiver;

diff --git a/mithril-client-cli/src/utils/progress_reporter.rs b/mithril-client-cli/src/utils/progress_reporter.rs
@@ -14,7 +14,7 @@ pub enum ProgressOutputType {
     /// Output to json
     JsonReporter,
     /// Output to tty
-    TTY,
+    Tty,
     /// No output
     Hidden,
 }
@@ -23,7 +23,7 @@ impl From<ProgressOutputType> for ProgressDrawTarget {
     fn from(value: ProgressOutputType) -> Self {
         match value {
             ProgressOutputType::JsonReporter => ProgressDrawTarget::hidden(),
-            ProgressOutputType::TTY => ProgressDrawTarget::stdout(),
+            ProgressOutputType::Tty => ProgressDrawTarget::stdout(),
             ProgressOutputType::Hidden => ProgressDrawTarget::hidden(),
         }
     }
@@ -54,7 +54,7 @@ impl ProgressPrinter {
                 timestamp = Utc::now().to_rfc3339(),
                 number_of_steps = self.number_of_steps,
             ),
-            ProgressOutputType::TTY => self
+            ProgressOutputType::Tty => self
                 .multi_progress
                 .println(format!("{step_number}/{} - {text}", self.number_of_steps))?,
             ProgressOutputType::Hidden => (),
@@ -72,9 +72,11 @@ impl Deref for ProgressPrinter {
     }
 }
 
+/// Utility to format a [ProgressBar] status as json
 pub struct ProgressBarJsonFormatter;
 
 impl ProgressBarJsonFormatter {
+    /// Get a json formatted string given the progress bar status
     pub fn format(progress_bar: &ProgressBar) -> String {
         format!(
             r#"{{"timestamp": "{}", "bytes_downloaded": {}, "bytes_total": {}, "seconds_left": {}.{:0>3}, "seconds_elapsed": {}.{:0>3}}}"#,
@@ -97,7 +99,7 @@ pub struct DownloadProgressReporter {
 }
 
 impl DownloadProgressReporter {
-    /// Instanciate a new progress reporter
+    /// Instantiate a new progress reporter
     pub fn new(progress_bar: ProgressBar, output_type: ProgressOutputType) -> Self {
         Self {
             progress_bar,
@@ -129,6 +131,7 @@ impl DownloadProgressReporter {
         };
     }
 
+    /// Report that the current download is finished and print the given message.
     pub fn finish(&self, message: &str) {
         self.progress_bar.finish_with_message(message.to_string());
     }

diff --git a/mithril-client-cli/src/utils/snapshot.rs b/mithril-client-cli/src/utils/snapshot.rs
@@ -67,21 +67,15 @@ mod test {
 
     #[test]
     fn check_disk_space_error_should_return_error_if_error_is_not_error_not_enough_space() {
-        let error = SnapshotUnpackerError::UnpackFailed {
-            dirpath: PathBuf::from(""),
-            error: anyhow::Error::msg("Some error message"),
-        };
+        let error = SnapshotUnpackerError::UnpackDirectoryAlreadyExists(PathBuf::new());
 
         let error = SnapshotUtils::check_disk_space_error(anyhow!(error))
             .expect_err("check_disk_space_error should fail");
 
         assert!(
             matches!(
                 error.downcast_ref::<SnapshotUnpackerError>(),
-                Some(SnapshotUnpackerError::UnpackFailed {
-                    dirpath: _,
-                    error: _
-                })
+                Some(SnapshotUnpackerError::UnpackDirectoryAlreadyExists(_))
             ),
             "Unexpected error: {:?}",
             error

diff --git a/mithril-client-cli/src/utils/unpacker.rs b/mithril-client-cli/src/utils/unpacker.rs
@@ -38,17 +38,6 @@ pub enum SnapshotUnpackerError {
     /// Cannot write in the given directory.
     #[error("Unpack directory '{0}' is not writable.")]
     UnpackDirectoryIsNotWritable(PathBuf, #[source] StdError),
-
-    /// Unpacking error
-    #[error("Could not unpack from streamed data snapshot to directory '{dirpath}'")]
-    UnpackFailed {
-        /// Location where the archive is to be extracted.
-        dirpath: PathBuf,
-
-        /// Subsystem error
-        #[source]
-        error: StdError,
-    },
 }
 
 impl SnapshotUnpacker {