Skip to content

DX Improvements#130

Closed
FletcherDares wants to merge 0 commit into
mainfrom
fletcher/dx-improvements
Closed

DX Improvements#130
FletcherDares wants to merge 0 commit into
mainfrom
fletcher/dx-improvements

Conversation

@FletcherDares
Copy link
Copy Markdown
Collaborator

No description provided.

let minute = ((total_seconds % 3600.0) / 60.0).floor() as i64;
let second_val = (total_seconds % 3600.0) % 60.0;
let second = second_val.floor() as i64;
let subsecond = second_val - second as f64;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The as_time method no longer outputs subsecond precision, even though to_calendar_components now extracts it. This is a change in behavior from the previous is_subsecond flag logic. If subsecond precision is required in string output, consider adding an argument or a separate method for it.

(year, month, day, hour, minute, second, subsecond)
}

pub fn as_date(&self) -> String {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to as_time, the as_datetime method also omits subsecond precision in its string output. If subsecond precision is needed for datetime string representations, this should be addressed.

second as f64,
subsecond,
))
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DateTimeModifier::Ceiling and DateTimeModifier::Floor variants are defined in modifiers.rs but are not handled in apply_modifier. This will result in a generic "Modifier not implemented" error if these are encountered. Consider implementing their logic or removing them if they are not intended for use.

@github-actions
Copy link
Copy Markdown

AI review done up to commit: 021a3d8

AI Review Summary:

Code review completed. Reviewed 2 files with 3 comments.

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