Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| @@ -0,0 +1,292 @@ | |||
| use std::collections::HashMap; | |||
There was a problem hiding this comment.
move into feedback crate?
codex-rs/tui/src/chatwidget.rs
Outdated
| ) { | ||
| // Build a fresh snapshot at the time of opening the note overlay. | ||
| let snapshot = self.feedback.snapshot(self.thread_id); | ||
| let diagnostics = codex_core::feedback_diagnostics::FeedbackDiagnostics::collect_from_env(); |
There was a problem hiding this comment.
should this be collected in feedback.snapshot ?
There was a problem hiding this comment.
feedback.snapshot returns a CodexLogSnapshot, do we want to include diagnostics in that? i guess the diagnostics are kind of a log snapshot but since they exist separately as env variables not sure if it makes sense if its not surely consistent? lmk
There was a problem hiding this comment.
you can rename the thing we return a FeedbackSnapshot and then it'll fit.
My goal is to have upload_feedback "just work" and upload everything including diagnostics.
|
|
||
| struct PreparedFeedbackAttachments { | ||
| attachment_paths: Vec<PathBuf>, | ||
| _diagnostics_attachment: Option<FeedbackDiagnosticsAttachment>, |
codex-rs/tui/src/chatwidget.rs
Outdated
| include_logs: bool, | ||
| diagnostics: codex_feedback::feedback_diagnostics::FeedbackDiagnostics, | ||
| ) { | ||
| // Build a fresh snapshot at the time of opening the note overlay. |
There was a problem hiding this comment.
codex_feedback::feedback_diagnostics::FeedbackDiagnostics::collect_from_env(); should run as part of feedback.snapshot
| Self::collect_from_pairs(std::env::vars()) | ||
| } | ||
|
|
||
| pub fn collect_from_pairs<I, K, V>(pairs: I) -> Self |
There was a problem hiding this comment.
this shouldn't be public
2 participants
Uh oh!
There was an error while loading. Please reload this page.