-
Notifications
You must be signed in to change notification settings - Fork 6.1k
chore: config editor #5878
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
chore: config editor #5878
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ 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". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful.
Three comments, neither blocking:
- Consider representing config update using a struct that mirrors Config struct. It becomes very clear what exactly is being updated and why.
- Let's assert before/after of config edits against raw strings to ensure formatting isn't messed up.
- Can pick either builder or edit array an use it consistently?
The goal is to have a single place where we actually write files
In a follow-up PR, will move everything config related in a dedicated module and move the helpers in a dedicated file