Skip to content

Conversation

@billyto
Copy link
Owner

@billyto billyto commented Jun 16, 2025

Adds more date formats, improves tests and readme

@billyto billyto requested a review from Copilot June 16, 2025 20:01
Copy link
Contributor

Copilot AI left a 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 week and year_week.
  • Updated CLI help, bumped version in Cargo.toml, and overhauled README.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/MM is paired with the example 19.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 24 but the CLI prints Is 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"));
Copy link

Copilot AI Jun 16, 2025

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.

Suggested change
.stdout(predicate::str::contains("Is weeek 7"));
.stdout(predicate::str::contains("Is week 7"));

Copilot uses AI. Check for mistakes.
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]"));
Copy link

Copilot AI Jun 16, 2025

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.

Suggested change
.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());

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@billyto billyto merged commit 5e2977f into main Jun 16, 2025
1 check passed
@billyto billyto deleted the v1.1 branch June 17, 2025 07:29
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.

2 participants