Skip to content

Conversation

@nekevss
Copy link
Member

@nekevss nekevss commented Sep 25, 2025

Put together this PR based on some ongoing discussion around the host system returned by Temporal::now().

I think this may be the right direction for this API in temporal_rs. But thought I'd at least put it together for further discussion.

@nekevss nekevss requested a review from a team September 25, 2025 04:23
@Manishearth
Copy link
Contributor

I think this is a good idea

@nekevss
Copy link
Member Author

nekevss commented Sep 25, 2025

I mentioned this on Matrix, but just so that it's visible here.

Thoughts on the actual method names?

For instance, in theory this could also be changed to.

struct Temporal;

impl Temporal {
    fn now_with_system_fallback() -> Now<LocalHostSystem> {
        todo!()
    }

    fn now_with_utc_fallback() -> Now<UtcHostSystem> {
        todo!()
    }
}

@Manishearth
Copy link
Contributor

This isn't fallback, is it? It's the system Now or the UTC Now?

@nekevss
Copy link
Member Author

nekevss commented Sep 25, 2025

Hmmmmm, that's true it's more of a host default value.

It's more so that if no TimeZone is provided, then we default / "fall back" to the host time zone. I'm not entirely convinced that that is a fall back, per se. But I think it conveys the behavior of how the Now will do in an explicit manner.

But calling it a fallback, then makes the time_zone method on Now incredibly strange, so we can go with the current names.

@nekevss nekevss marked this pull request as ready for review September 26, 2025 00:30
@nekevss nekevss requested a review from Manishearth September 26, 2025 03:53
@nekevss nekevss added the C-api Changes related to the public API label Dec 17, 2025
@nekevss nekevss merged commit 5714994 into main Dec 23, 2025
8 checks passed
@nekevss nekevss deleted the rework-now branch December 23, 2025 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-api Changes related to the public API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants