Skip to content

Conversation

@peter-jerry-ye
Copy link
Collaborator

No description provided.

@peter-jerry-ye-code-review
Copy link

peter-jerry-ye-code-review bot commented May 28, 2025

Missing comprehensive documentation for public API

Category
Maintainability
Code Snippet
fn log(String, &Show, @types.Level) -> Unit
fn set_level(@types.Level) -> Unit
Recommendation
Add detailed documentation comments explaining parameter roles and usage:

/// Log a message with given level
/// @param message The message to log
/// @param value The value to show
/// @param level The logging level
fn log(String, &Show, @types.Level) -> Unit

Reasoning
Public APIs should have clear documentation to help users understand the purpose and usage of each parameter. This is especially important for a logging library that will be widely used.

Log function parameter names are not descriptive

Category
Maintainability
Code Snippet
fn log(String, &Show, @types.Level) -> Unit
Recommendation
Use descriptive parameter names:

fn log(message: String, value: &Show, level: @types.Level) -> Unit

Reasoning
Named parameters make the code more self-documenting and easier to understand when reading function calls. It's particularly important for public APIs.

Level enum lacks descriptions for each level value

Category
Maintainability
Code Snippet
pub(all) enum Level {
Debug
Info
...
}
Recommendation
Add documentation for each level:

pub(all) enum Level {
  /// Detailed debug information
  Debug
  /// General informational messages
  Info
  ...
}

Reasoning
Documenting enum variants helps users choose the appropriate logging level. RFC 5424 defines specific meanings for each level that should be documented.

@peter-jerry-ye peter-jerry-ye force-pushed the zihang/virtual-logging branch from b26c8f3 to 7e371bf Compare May 29, 2025 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant