Skip to content

remove-gulp#605

Merged
yaacov merged 1 commit intomainfrom
remove-gulp
Mar 20, 2026
Merged

remove-gulp#605
yaacov merged 1 commit intomainfrom
remove-gulp

Conversation

@yaacov
Copy link
Copy Markdown
Owner

@yaacov yaacov commented Mar 20, 2026

Summary by CodeRabbit

  • Chores
    • Modernized build system: replaced Gulp-based workflow with npm scripts (clean, build, doc, docs, publish:prepare).
    • Updated development dependencies: removed Gulp-related packages, added JSDoc and documentation theme.
    • Configured JSDoc for automated documentation generation from source code.
    • Enhanced linting configuration to exclude generated artifact directories.

Signed-off-by: yaacov <kobi.zamir@gmail.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

The PR migrates the project's build system from Gulp to npm scripts, removing gulpfile.js and introducing three new build scripts (clean.js, build-modbus-serial.js, copy-doc-examples.js). A jsdoc.json configuration is added, package.json is updated with new npm scripts and dependencies, ESLint's ignore paths are extended, and JSDoc type annotations are refined.

Changes

Cohort / File(s) Summary
Build System Migration
gulpfile.js, package.json, scripts/build-modbus-serial.js, scripts/clean.js, scripts/copy-doc-examples.js
Removes Gulp-based workflow entirely. Adds three new npm scripts handling directory cleanup, copying source directories to modbus-serial/, and copying documentation examples. Updates package.json to expose clean, build, doc, doc:examples, docs, and publish:prepare npm scripts; replaces gulp, gulp-clean, gulp-jsdoc3, and pump with jsdoc and clean-jsdoc-theme.
Documentation Configuration
jsdoc.json, README.md
Adds new JSDoc configuration specifying source paths, output directory (./docs/gen), markdown plugin, and theme settings. README documents the new npm scripts and their composition relationships.
Linting Configuration
eslint.config.mjs
Extends ESLint's top-level ignores list to exclude **/docs/** and **/modbus-serial/** directories from linting.
Code Documentation
ports/tcpport.js
Refines JSDoc type annotations for constructor options and callback parameters without altering runtime logic or control flow.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • update-eslint-rules #604: Modifies eslint.config.mjs with similar ignore list extensions, directly overlapping configuration changes.
  • fix-lint #603: Updates both eslint.config.mjs and gulpfile.js in a related lint/build orchestration effort, sharing affected files and overall modernization intent.

Poem

🐰 The gulp has been tucked away with care,
npm scripts now dance through the air!
Clean, build, and docs in harmony align,
Jsdoc themes make docs so fine—
A simpler path, no more complex affairs! 📚

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'remove-gulp' accurately summarizes the main change—removing Gulp build infrastructure and replacing it with npm scripts.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove-gulp
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
scripts/build-modbus-serial.js (1)

10-15: Build script creates incomplete mirrored tree — index.js and root-level TypeScript definitions are missing.

The README documents that npm run build should create a "mirrored package tree" in modbus-serial/, but the script currently copies only subdirectories (apis, ports, servers, utils) without the main entry point index.js or TypeScript definition files. If modbus-serial/ is meant to be a complete mirror as documented, it should also include root-level files that define the public API.

Note: The root package.json (main package) uses index.js from the repository root, so the npm package itself is functional. However, the build output is incomplete relative to its documented purpose.

Proposed fix to complete the mirrored tree
 for (const name of ["apis", "ports", "servers", "utils"]) {
     const src = path.join(root, name);
     const dest = path.join(out, name);
     fs.rmSync(dest, { recursive: true, force: true });
     fs.cpSync(src, dest, { recursive: true });
 }
+
+// Copy entry point and type definitions
+for (const file of ["index.js", "index.d.ts"]) {
+    const src = path.join(root, file);
+    if (fs.existsSync(src)) {
+        fs.copyFileSync(src, path.join(out, file));
+    }
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/build-modbus-serial.js` around lines 10 - 15, The build loop that
copies only the subfolders (the for loop using names, src, dest, fs.rmSync and
fs.cpSync) misses root-level public files — copy the repository root index.js
plus any root-level TypeScript declaration files (e.g., *.d.ts) into the output
mirror (out/modbus-serial) so the mirrored package exposes the same public API;
update the script after the existing loop to rmSync the output root file(s) and
cpSync the source index.js and root-level .d.ts files (and any other top-level
public files like package.json if the mirror should be complete) into the out
directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@scripts/build-modbus-serial.js`:
- Around line 10-15: The build loop that copies only the subfolders (the for
loop using names, src, dest, fs.rmSync and fs.cpSync) misses root-level public
files — copy the repository root index.js plus any root-level TypeScript
declaration files (e.g., *.d.ts) into the output mirror (out/modbus-serial) so
the mirrored package exposes the same public API; update the script after the
existing loop to rmSync the output root file(s) and cpSync the source index.js
and root-level .d.ts files (and any other top-level public files like
package.json if the mirror should be complete) into the out directory.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: be2b7452-2b33-4928-88dd-74f8d01a3e71

📥 Commits

Reviewing files that changed from the base of the PR and between 44d2c52 and 3bc9691.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • README.md
  • eslint.config.mjs
  • gulpfile.js
  • jsdoc.json
  • package.json
  • ports/tcpport.js
  • scripts/build-modbus-serial.js
  • scripts/clean.js
  • scripts/copy-doc-examples.js
💤 Files with no reviewable changes (1)
  • gulpfile.js

@yaacov yaacov merged commit 8b7ff06 into main Mar 20, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant