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

System time implementation #4

Merged
merged 5 commits into from
Apr 22, 2023
Merged

System time implementation #4

merged 5 commits into from
Apr 22, 2023

Conversation

museun
Copy link
Owner

@museun museun commented Apr 20, 2023

This is discussed in #3

Copy link

@bryanlarsen bryanlarsen left a comment

Choose a reason for hiding this comment

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

This works in my tests. You definitely put more work into this than I expected. Highly appreciated! Thank you.

src/lib.rs Outdated
@@ -50,6 +73,16 @@ mod reference {
pub fn get_time() -> Duration {
*TIME.get_or_init(Mutex::default).lock().unwrap()
}

pub fn get_time() -> Duration {

Choose a reason for hiding this comment

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

there are two get_time functions here. I assume one will alias the other.

src/lib.rs Outdated
pub struct SystemTime(Duration);

impl SystemTime {
pub const UNIX_EPOCH: SystemTime = SystemTime(Duration::from_secs(0));

Choose a reason for hiding this comment

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

Being inside the struct prevents me from accessing it via use the way I access std::time::UNIX_EPOCH;

I worked around it with

#[cfg(test)]
const UNIX_EPOCH: SystemTime = SystemTime::UNIX_EPOCH;

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oh, one should be with_system_time. copy-pasted error (basically get-nnn is a read-only accessor, and with-nnn are a way to modify the value atomically)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Being inside the struct prevents me from accessing it via use the way I access std::time::UNIX_EPOCH;

I worked around it with

#[cfg(test)]
const UNIX_EPOCH: SystemTime = SystemTime::UNIX_EPOCH;

Interesting, I didn't know UNIX_EPOCH was at the module level. std has the struct-level one aliased from the module level one. I added this in 07c1ab2

@museun
Copy link
Owner Author

museun commented Apr 20, 2023

If everything looks good now, I'll merge and publish it.

@museun museun merged commit e9dd947 into master Apr 22, 2023
@museun museun deleted the system-time branch June 3, 2023 05:30
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