fix(core): enable history lookup on windows #7457
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
|
@codex Review |
- Add portable history log id helper to support inode-like tracking on Unix and creation time on Windows - Refactor history metadata and lookup to share code paths and allow nonzero log ids across platforms - Add coverage for lookup stability after appends
stevemostovoy-openai
force-pushed
the
steve/windows-message-history
branch
from
e6d083e to
6610af4
Compare
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
dylan-hurd-oai
left a comment
dylan-hurd-oai
left a comment
There was a problem hiding this comment.
Requesting some code cleanup, but this approach feels reasonable to me!
codex-rs/core/src/message_history.rs
Outdated
| None | ||
| fn history_log_id(metadata: &std::fs::Metadata) -> Option<u64> { | ||
| if cfg!(windows) { | ||
| use std::os::windows::fs::MetadataExt; |
There was a problem hiding this comment.
ah overlooked this import - @stevemostovoy-openai I think we do need to use a #[cfg(windows)] / #[cfg(not(windows))] macro here
Uh oh!
There was an error while loading. Please reload this page.