feat(cli): add footers empty rule #327
Conversation
Signed-off-by: KeisukeYamashita <19yamashita15@gmail.com>
Signed-off-by: KeisukeYamashita <19yamashita15@gmail.com>
WalkthroughThe recent updates enhance the commit message validation system by introducing a new rule for ensuring footers are included in messages. A dedicated module and struct for footer validation have been added, along with a test for parsing multiline commit messages. Documentation for the new rule clarifies expectations for developers, promoting best practices in commit message formatting. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant CommitMessageParser
participant FooterValidator
Developer->>CommitMessageParser: Submit commit message
CommitMessageParser->>FooterValidator: Validate footers
FooterValidator-->>CommitMessageParser: Return validation result
CommitMessageParser-->>Developer: Provide feedback
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (5)
web/src/content/docs/rules/footers-empty.md (3)
3-3: Clarify the description.The description "Check if the footers exists" should be more precise.
- description: Check if the footers exists + description: Ensure that footers are present in commit messages
10-12: Provide a more realistic bad example.The bad example should illustrate a commit message where a footer is expected but missing.
- feat(cli): user logout handler + feat(cli): add new flag + + This is a description of the commit.
17-20: Enhance the good example.The good example should include a more comprehensive commit message.
- feat(cli): add new flag - - Link: https://keisukeyamashita.github.io/commitlint-rs/ + feat(cli): add new flag for user logout + + This commit adds a new flag for the user logout functionality. + + Link: https://keisukeyamashita.github.io/commitlint-rs/src/rule/footers_empty.rs (2)
1-14: Consider adding documentation for thelevelfield.The
levelfield lacks documentation. Adding a comment would improve clarity./// Level represents the level of the rule. /// // Note that currently the default literal is not supported. // See: https://github.com/serde-rs/serde/issues/368 level: Option<Level>, + /// The level of severity for the rule. If not specified, defaults to `Error`.
16-35: Ensure thevalidatemethod handles edge cases.The
validatemethod should handle cases wherefootersis an empty map.
- Current code only checks if
footersisNone.- Update the
validatemethod to also check iffootersisSomebut empty.fn validate(&self, message: &Message) -> Option<Violation> { if message.footers.is_none() || message.footers.as_ref().unwrap().is_empty() { return Some(Violation { level: self.level.unwrap_or(Self::LEVEL), message: self.message(message), }); } None }
- File:
src/rule/footers_empty.rs- Lines: 16-35
Analysis chain
Ensure the
validatemethod handles edge cases.The
validatemethod should handle cases wherefootersis an empty map.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `validate` method handles cases where `footers` is an empty map. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type rust -A 5 $'fn validate'Length of output: 7276
Script:
#!/bin/bash # Description: Verify the definition of the `Message` struct and its `footers` field. # Search for the definition of the `Message` struct. rg --type rust 'struct Message' # Search for the definition of the `footers` field within the `Message` struct. rg --type rust 'footers' -A 3Length of output: 15605
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/git.rs (1 hunks)
- src/rule.rs (3 hunks)
- src/rule/footers_empty.rs (1 hunks)
- web/src/content/docs/rules/footers-empty.md (1 hunks)
Additional comments not posted (5)
web/src/content/docs/rules/footers-empty.md (1)
26-30: Ensure YAML configuration is correct.The YAML configuration example is correct and clear.
src/rule.rs (3)
9-11: Ensure the module declaration is consistent.The module declaration is consistent with other modules.
54-56: Ensure consistency in field naming.The field naming is consistent with other fields in the struct.
198-198: Ensure the default implementation is correct.The default implementation is correct and follows best practices.
src/git.rs (1)
178-195: LGTM!The new test function
test_footer_with_multiline_body_parse_commit_messageis well-structured and adds valuable coverage for parsing multiline commit messages. The assertions are clear and correctly structured.
| #[cfg(test)] | ||
| mod tests { | ||
| use std::collections::HashMap; | ||
|
|
||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn test_non_empty_footer() { | ||
| let rule = FootersEmpty::default(); | ||
|
|
||
| let mut f = HashMap::new(); | ||
| f.insert("Link".to_string(), "hello".to_string()); | ||
|
|
||
| let message = Message { | ||
| body: Some("Hello world".to_string()), | ||
| description: Some("broadcast $destroy event on scope destruction".to_string()), | ||
| footers: Some(f), | ||
| r#type: Some("feat".to_string()), | ||
| raw: "feat(scope): broadcast $destroy event on scope destruction | ||
|
|
||
| Hello world | ||
|
|
||
| Link: hello" | ||
| .to_string(), | ||
| scope: Some("scope".to_string()), | ||
| subject: Some("feat(scope): broadcast $destroy event on scope destruction".to_string()), | ||
| }; | ||
|
|
||
| assert!(rule.validate(&message).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_empty_footer() { | ||
| let rule = FootersEmpty::default(); | ||
| let message = Message { | ||
| body: None, | ||
| description: None, | ||
| footers: None, | ||
| r#type: Some("feat".to_string()), | ||
| raw: "feat(scope): broadcast $destroy event on scope destruction".to_string(), | ||
| scope: Some("scope".to_string()), | ||
| subject: None, | ||
| }; | ||
|
|
||
| let violation = rule.validate(&message); | ||
| assert!(violation.is_some()); | ||
| assert_eq!(violation.clone().unwrap().level, Level::Error); | ||
| assert_eq!(violation.unwrap().message, "footers are empty".to_string()); | ||
| } |
There was a problem hiding this comment.
Consider adding more test cases.
The tests cover basic cases. Consider adding tests for edge cases, such as when footers is an empty map.
#[test]
fn test_empty_footer_map() {
let rule = FootersEmpty::default();
let message = Message {
body: None,
description: None,
footers: Some(HashMap::new()),
r#type: Some("feat".to_string()),
raw: "feat(scope): broadcast $destroy event on scope destruction".to_string(),
scope: Some("scope".to_string()),
subject: None,
};
let violation = rule.validate(&message);
assert!(violation.is_some());
assert_eq!(violation.clone().unwrap().level, Level::Error);
assert_eq!(violation.unwrap().message, "footers are empty".to_string());
}
Why
Because in some companies, the commit author attaches some links to a commit.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes