From a1d26148c4bbf20ca6d77bb72e27a01cb5bb563a Mon Sep 17 00:00:00 2001 From: khai96_ Date: Tue, 8 Aug 2023 12:52:57 +0700 Subject: [PATCH 1/5] refactor: one less `.expect`, one less panic --- crates/diagnostics/src/local_tracing.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/diagnostics/src/local_tracing.rs b/crates/diagnostics/src/local_tracing.rs index 3da11835b..e2eefaecf 100644 --- a/crates/diagnostics/src/local_tracing.rs +++ b/crates/diagnostics/src/local_tracing.rs @@ -6,12 +6,13 @@ use tracing_subscriber::{fmt::format::FmtSpan, EnvFilter, Layer}; static IS_TRACING_ENABLED: AtomicBool = AtomicBool::new(false); pub fn enable_tracing_by_env() { - let trace_var = std::env::var("TRACE").ok(); - let is_enable_tracing = trace_var.is_some(); + let Ok(trace_var) = std::env::var("TRACE") else { + return + }; - if is_enable_tracing && !IS_TRACING_ENABLED.swap(true, std::sync::atomic::Ordering::SeqCst) { + if !IS_TRACING_ENABLED.swap(true, std::sync::atomic::Ordering::SeqCst) { use tracing_subscriber::{fmt, prelude::*}; - let layers = generate_common_layers(trace_var); + let layers = generate_common_layers(&trace_var); tracing_subscriber::registry() .with(layers) @@ -22,24 +23,21 @@ pub fn enable_tracing_by_env() { } fn generate_common_layers( - trace_var: Option, + trace_var: &str, ) -> Vec + Send + Sync>> { - let default_level = trace_var.as_ref().and_then(|var| Level::from_str(var).ok()); - let mut layers = vec![]; - if let Some(default_level) = default_level { + if let Ok(default_level) = Level::from_str(trace_var) { layers.push( tracing_subscriber::filter::Targets::new() .with_targets(vec![("pacquet_tarball", default_level)]) .boxed(), ); } else { - // SAFETY: we know that trace_var is `Ok(String)` now, - // for the second unwrap, if we can't parse the directive, then the tracing result would be + // SAFETY: for the `expect`, if we can't parse the directive, then the tracing result would be // unexpected, then panic is reasonable let env_layer = EnvFilter::builder() .with_regex(true) - .parse(trace_var.expect("Should not be empty")) + .parse(trace_var) .expect("Parse tracing directive syntax failed,for details about the directive syntax you could refer https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html#directives"); layers.push(env_layer.boxed()); From bf249af0fe9defcd7818c387ca85dc781e7751ba Mon Sep 17 00:00:00 2001 From: khai96_ Date: Tue, 8 Aug 2023 13:05:08 +0700 Subject: [PATCH 2/5] refactor: remove unnecessary vec --- crates/diagnostics/src/local_tracing.rs | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/crates/diagnostics/src/local_tracing.rs b/crates/diagnostics/src/local_tracing.rs index e2eefaecf..47a93a4ed 100644 --- a/crates/diagnostics/src/local_tracing.rs +++ b/crates/diagnostics/src/local_tracing.rs @@ -12,35 +12,28 @@ pub fn enable_tracing_by_env() { if !IS_TRACING_ENABLED.swap(true, std::sync::atomic::Ordering::SeqCst) { use tracing_subscriber::{fmt, prelude::*}; - let layers = generate_common_layers(&trace_var); + let layer = common_layer(&trace_var); tracing_subscriber::registry() - .with(layers) + .with(layer) .with(fmt::layer().pretty().with_file(true).with_span_events(FmtSpan::CLOSE)) .init(); tracing::trace!("enable_tracing_by_env"); } } -fn generate_common_layers( - trace_var: &str, -) -> Vec + Send + Sync>> { - let mut layers = vec![]; +fn common_layer(trace_var: &str) -> Box + Send + Sync> { if let Ok(default_level) = Level::from_str(trace_var) { - layers.push( - tracing_subscriber::filter::Targets::new() - .with_targets(vec![("pacquet_tarball", default_level)]) - .boxed(), - ); + tracing_subscriber::filter::Targets::new() + .with_targets(vec![("pacquet_tarball", default_level)]) + .boxed() } else { // SAFETY: for the `expect`, if we can't parse the directive, then the tracing result would be // unexpected, then panic is reasonable - let env_layer = EnvFilter::builder() + EnvFilter::builder() .with_regex(true) .parse(trace_var) - .expect("Parse tracing directive syntax failed,for details about the directive syntax you could refer https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html#directives"); - - layers.push(env_layer.boxed()); + .expect("Parse tracing directive syntax failed,for details about the directive syntax you could refer https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html#directives") + .boxed() } - layers } From 56d62707b7435092c32c0d24e49d61ab1e0d1bfe Mon Sep 17 00:00:00 2001 From: khai96_ Date: Tue, 8 Aug 2023 13:06:21 +0700 Subject: [PATCH 3/5] refactor: remove unnecessary vec --- crates/diagnostics/src/local_tracing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/diagnostics/src/local_tracing.rs b/crates/diagnostics/src/local_tracing.rs index 47a93a4ed..49935dc41 100644 --- a/crates/diagnostics/src/local_tracing.rs +++ b/crates/diagnostics/src/local_tracing.rs @@ -25,7 +25,7 @@ pub fn enable_tracing_by_env() { fn common_layer(trace_var: &str) -> Box + Send + Sync> { if let Ok(default_level) = Level::from_str(trace_var) { tracing_subscriber::filter::Targets::new() - .with_targets(vec![("pacquet_tarball", default_level)]) + .with_targets([("pacquet_tarball", default_level)]) .boxed() } else { // SAFETY: for the `expect`, if we can't parse the directive, then the tracing result would be From d958c1998986322b45b70878174f2359b7cc8aed Mon Sep 17 00:00:00 2001 From: khai96_ Date: Tue, 8 Aug 2023 13:07:24 +0700 Subject: [PATCH 4/5] refactor: simplify --- crates/diagnostics/src/local_tracing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/diagnostics/src/local_tracing.rs b/crates/diagnostics/src/local_tracing.rs index 49935dc41..f2848ffaa 100644 --- a/crates/diagnostics/src/local_tracing.rs +++ b/crates/diagnostics/src/local_tracing.rs @@ -25,7 +25,7 @@ pub fn enable_tracing_by_env() { fn common_layer(trace_var: &str) -> Box + Send + Sync> { if let Ok(default_level) = Level::from_str(trace_var) { tracing_subscriber::filter::Targets::new() - .with_targets([("pacquet_tarball", default_level)]) + .with_target("pacquet_tarball", default_level) .boxed() } else { // SAFETY: for the `expect`, if we can't parse the directive, then the tracing result would be From 8c3695b18c46792ff7ece62a7cbe412a9cedb3a7 Mon Sep 17 00:00:00 2001 From: khai96_ Date: Tue, 8 Aug 2023 13:44:50 +0700 Subject: [PATCH 5/5] refactor: one less `.unwrap()`, one less panic --- crates/package_json/src/lib.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/package_json/src/lib.rs b/crates/package_json/src/lib.rs index 3f3004f8f..f83f8c591 100644 --- a/crates/package_json/src/lib.rs +++ b/crates/package_json/src/lib.rs @@ -1,7 +1,6 @@ use std::{ collections::HashMap, convert::Into, - ffi::OsStr, fs, io::{Read, Write}, path::PathBuf, @@ -77,11 +76,11 @@ impl PackageJson { fn write_to_file(path: &PathBuf) -> Result { let mut file = fs::File::create(path)?; - let mut name = ""; - if let Some(folder) = path.parent() { - // Set the default package name as the folder of the current directory - name = folder.file_name().unwrap_or(OsStr::new("")).to_str().unwrap(); - } + let name = path + .parent() + .and_then(|folder| folder.file_name()) + .and_then(|file_name| file_name.to_str()) + .unwrap_or(""); let package_json = json!({ "name": name, "version": "1.0.0",