Repository navigation
Conversation
Remove superseded JSON configs after their TypeScript replacements are written, and include generated root tool configs in framework tsconfig projects.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the configuration management lifecycle within the wbfy package. It ensures that legacy JSON-based linting and formatting configurations are cleaned up only after successful migration to TypeScript-based equivalents, preventing potential configuration gaps. Additionally, it enhances TypeScript project inclusion rules to better support generated root tool configurations. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Old JSON files now fade away, / As TypeScript takes the light of day. / With configs kept in proper place, / The project runs with cleaner grace. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the wbfy generators to safely remove legacy .oxfmtrc.json and .oxlintrc.json configuration files only after their TypeScript replacements are successfully generated. It also updates the framework tsconfig.json generator to include *.config.ts in the include array. The review feedback points out that unconditionally sorting the include array in tsconfig.json can trigger unnecessary file rewrites, which would strip comments and custom formatting. It suggests only sorting the array when a new element is actually added.
| function addManagedIncludesForFrameworkProject(settings: TsConfigJson): void { | ||
| // Omitting include lets framework tsconfigs keep TypeScript's default | ||
| // "all TS/TSX files" behavior, which already covers scripts. | ||
| // "all TS/TSX files" behavior, which already covers both paths. | ||
| if (!settings.include) return; | ||
| if (settings.include.includes('scripts/**/*')) return; | ||
|
|
||
| settings.include.push('scripts/**/*'); | ||
| for (const managedInclude of ['*.config.ts', 'scripts/**/*']) { | ||
| if (!settings.include.includes(managedInclude)) settings.include.push(managedInclude); | ||
| } | ||
| settings.include.sort(); | ||
| } |
There was a problem hiding this comment.
Unconditionally sorting settings.include will modify the array's order even if no new elements were added. This causes JSON.stringify(settings) === originalSettingsJson to evaluate to false if the original include array was unsorted, triggering an unnecessary rewrite of tsconfig.json and stripping all JSONC comments and custom formatting.\n\nBy only sorting when an element is actually added, we can prevent unnecessary writes and preserve comments/formatting in already-clean tsconfig.json files.
| function addManagedIncludesForFrameworkProject(settings: TsConfigJson): void { | |
| // Omitting include lets framework tsconfigs keep TypeScript's default | |
| // "all TS/TSX files" behavior, which already covers scripts. | |
| // "all TS/TSX files" behavior, which already covers both paths. | |
| if (!settings.include) return; | |
| if (settings.include.includes('scripts/**/*')) return; | |
| settings.include.push('scripts/**/*'); | |
| for (const managedInclude of ['*.config.ts', 'scripts/**/*']) { | |
| if (!settings.include.includes(managedInclude)) settings.include.push(managedInclude); | |
| } | |
| settings.include.sort(); | |
| } | |
| function addManagedIncludesForFrameworkProject(settings: TsConfigJson): void {\n // Omitting include lets framework tsconfigs keep TypeScript's default\n // \"all TS/TSX files\" behavior, which already covers both paths.\n if (!settings.include) return;\n\n let added = false;\n for (const managedInclude of ['*.config.ts', 'scripts/**/*']) {\n if (!settings.include.includes(managedInclude)) {\n settings.include.push(managedInclude);\n added = true;\n }\n }\n if (added) {\n settings.include.sort();\n }\n} |
References
- Simplify code as much as possible to eliminate redundancy. (link)
Keep legacy configs when managed blocks are incomplete, document and log canonical config replacement, and ensure framework projects type generated tool configs without needless JSONC rewrites.
Summary
Verification
WB_ENV=test bun run verify-fullbun start /Users/exkazuu/ghq/github.com/exKAZUu-Research/trace-dojofrompackages/wbfy