Skip to content
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

fix: replace deprecated API in chrono 0.4.35 for converting to time #367

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bconn98
Copy link
Collaborator

@bconn98 bconn98 commented Apr 4, 2024

Replace deprecated time conversion API from chrono crate.

Deprecation occured in v0.4.35.
Work performed by @Dirreke
Separated out by @bconn98
Resolves #366

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 81.25000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 63.45%. Comparing base (f688e38) to head (ad20abc).

Files Patch % Lines
...ppend/rolling_file/policy/compound/trigger/time.rs 81.25% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #367      +/-   ##
==========================================
+ Coverage   63.42%   63.45%   +0.03%     
==========================================
  Files          25       25              
  Lines        1572     1579       +7     
==========================================
+ Hits          997     1002       +5     
- Misses        575      577       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Dirreke
Copy link
Contributor

Dirreke commented Apr 4, 2024

and we can add this test

    #[test]
    fn trigger_large_interval() {
        let interval = TimeTriggerInterval::Second(i64::MAX);
        let file = tempfile::tempdir().unwrap();
        let logfile = LogFile {
            writer: &mut None,
            path: file.path(),
            len: 0,
        };
        let config = TimeTriggerConfig {
            interval,
            ..Default::default()
        };

        let trigger = TimeTrigger::new(config);
        let error = trigger.trigger(&logfile).unwrap_err();
        let box_dyn = Box::<dyn StdError>::from(error);
        assert_eq!(
            format!("The integer value {:?} for the specified time trigger interval is too large, it must be less than 9,223,372,036,854,775,807 seconds.", interval),
            box_dyn.to_string()
        );
    }

@bconn98
Copy link
Collaborator Author

bconn98 commented Apr 4, 2024

and we can add this test

@Dirreke Doesn't look like it's doable without some of your refactor. Based on needing to unwrap the error in the new method vs unwrapping in the trigger. If you see another option let me know. I messed around with catch_unwind but without a concrete error type that wasn't working for me.

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.

Resolve Deprecated Chrono API
3 participants