refactor: build js/util code from src/util code#87
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe pull request adds command-history functionality to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tsconfig.util.json (1)
13-13: Consider if recursive glob is needed.The pattern
src/util/*.tsonly matches files directly insrc/util/, not nested subdirectories. If utility files may be organized into subfolders in the future, consider usingsrc/util/**/*.tsinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.util.json` at line 13, The tsconfig include uses a non-recursive glob "src/util/*.ts" which only matches files directly under src/util; if utilities may live in subfolders, update the include pattern to a recursive glob such as "src/util/**/*.ts" so nested .ts files are picked up by the compiler and tooling (replace the existing "src/util/*.ts" entry in tsconfig.util.json).js/console.js (1)
1-418: Consider documenting the dual-maintenance requirement.
js/console.jsandsrc/console.tsimplement identical command-history logic but are maintained independently (different build outputs, different feature sets likeImprovDialog). Future bug fixes or enhancements to the history feature must be applied to both files.Consider adding a code comment near the history fields referencing the other file to help future maintainers keep them synchronized.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/console.js` around lines 1 - 418, Add a short code comment above the command-history fields (commandHistory, historyIndex, currentInput) in ESP32ToolConsole noting that the history implementation is duplicated in src/console.ts and must be updated in both places; include the other file name and a brief note like "keep in sync with src/console.ts" so future maintainers see the dual-maintenance requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@js/console.js`:
- Around line 1-418: Add a short code comment above the command-history fields
(commandHistory, historyIndex, currentInput) in ESP32ToolConsole noting that the
history implementation is duplicated in src/console.ts and must be updated in
both places; include the other file name and a brief note like "keep in sync
with src/console.ts" so future maintainers see the dual-maintenance requirement.
In `@tsconfig.util.json`:
- Line 13: The tsconfig include uses a non-recursive glob "src/util/*.ts" which
only matches files directly under src/util; if utilities may live in subfolders,
update the include pattern to a recursive glob such as "src/util/**/*.ts" so
nested .ts files are picked up by the compiler and tooling (replace the existing
"src/util/*.ts" entry in tsconfig.util.json).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b47947c-2f02-4d54-bc30-94d5eb3843cb
⛔ Files ignored due to path filters (11)
apple-touch-icon.pngis excluded by!**/*.pngicons/icon-128.pngis excluded by!**/*.pngicons/icon-144.pngis excluded by!**/*.pngicons/icon-152.pngis excluded by!**/*.pngicons/icon-192.pngis excluded by!**/*.pngicons/icon-384.pngis excluded by!**/*.pngicons/icon-512.pngis excluded by!**/*.pngicons/icon-72.pngis excluded by!**/*.pngicons/icon-96.pngis excluded by!**/*.pngscreenshots/desktop.pngis excluded by!**/*.pngscreenshots/mobile.pngis excluded by!**/*.png
📒 Files selected for processing (7)
js/console.jsjs/util/console-color.jsjs/util/line-break-transformer.jsjs/util/timestamp-transformer.jspackage.jsonsrc/console.tstsconfig.util.json
|
@coderabbitai the changes in package.json and tsconfig.util.json are the interesting in this PR. Does build the js/util from the source code src/util without issues? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me inspect the relevant files to give you a thorough answer. 🏁 Script executed: Length of output: 17754
✅ What works correctly
|
Summary by CodeRabbit
New Features
Style