-
Notifications
You must be signed in to change notification settings - Fork 0
V1.1 #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR expands date format support, enhances testing coverage, and updates project metadata and documentation.
- Added parsing logic for various date formats including yearless dates.
- Improved and expanded unit and integration tests for both
weekandyear_week. - Updated CLI help, bumped version in
Cargo.toml, and overhauledREADME.md.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| week/src/lib.rs | Refactored parsing into week and yearless_week, added tests. |
| week/src/main.rs | Clarified CLI help, improved error context, updated output format. |
| week/tests/cli_tests.rs | Replaced and expanded integration tests covering many scenarios. |
| week/tests/cli.rs | Removed outdated test file. |
| week/Cargo.toml | Bumped version, added metadata, updated dependencies. |
| README.md | Extended usage examples, formats table, roadmap, and badges. |
Comments suppressed due to low confidence (5)
week/src/main.rs:49
- There's a typo in the output string:
"Is weeek {}`` should be"Is week {}``.
println!("Is weeek {}", week_of_year);
week/src/main.rs:29
- The listed format
DD/MMis paired with the example19.02. That’s inconsistent. Update to match (e.g.,DD.MM (19.02)or correct the example).
/// - DD/MM (e.g., 19.02) - uses current year
week/src/main.rs:30
- [nitpick] This duplicates the previous line. Consider consolidating the list of supported formats to avoid confusion.
/// - DD/MM (e.g., 19/02) - uses current year
README.md:35
- Example output shows
Week 24but the CLI printsIs week 24. Align documentation with actual output or adjust the program output.
$ week
week/src/lib.rs:80
- [nitpick] This assertion message is duplicated for both slash and dot formats. Update the message for the dot case to "with dot" for clarity.
assert!(result.is_ok(), "Should handle yearless format with slash");
| cmd.arg("--date").arg("19-02-2023"); | ||
| cmd.assert() | ||
| .success() | ||
| .stdout(predicate::str::contains("Is weeek 7")); |
Copilot
AI
Jun 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Contains the same typo weeek. Adjust to Is week 7 after fixing the CLI output.
| .stdout(predicate::str::contains("Is weeek 7")); | |
| .stdout(predicate::str::contains("Is week 7")); |
| cmd.arg("--date").arg("01/01"); | ||
| cmd.assert() | ||
| .success() | ||
| .stdout(predicate::str::is_match("Is weeek \\d{1,2}\\n").unwrap()); //.stdout(predicate::str::is_match("[0-9][0-9]")); |
Copilot
AI
Jun 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Remove this commented-out assertion to keep the test suite clean.
| .stdout(predicate::str::is_match("Is weeek \\d{1,2}\\n").unwrap()); //.stdout(predicate::str::is_match("[0-9][0-9]")); | |
| .stdout(predicate::str::is_match("Is weeek \\d{1,2}\\n").unwrap()); |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Adds more date formats, improves tests and readme