Skip to content

refactor: use filesystem promises to clear lint warnings - #148

Open
fengmk2 wants to merge 1 commit into
masterfrom
codex/lint-cleanup
Open

fengmk2 wants to merge 1 commit into
masterfrom
codex/lint-cleanup

Conversation

@fengmk2

@fengmk2 fengmk2 commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

node/prefer-promises/fs reports warnings for callback-based filesystem operations. Use fs.promises while keeping stream errors and extraction sequencing intact. Correct @returns to @return to satisfy jsdoc/check-tag-names.

Summary by CodeRabbit

  • Refactor
    • Updated archive file and directory handling to use promise-based filesystem operations. File metadata lookup, directory creation, and directory enumeration continue to report failures through the existing error-handling flows.
  • Tests
    • Updated stream-uncompression tests to wait for directory creation to complete before continuing, and to report creation failures.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 380e24f0-5a17-4491-aae8-8d708d2efb12
Commits

Reviewing files that changed from the base of the PR and between 806be7f and 33d6be1.

Files selected for processing: 6
  • lib/tar/file_stream.js
  • lib/tar/stream.js
  • lib/utils.js
  • test/tar/uncompress_stream.test.js
  • test/tgz/uncompress_stream.test.js
  • test/zip/uncompress_stream.test.js

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

Tar processing and extraction setup now use promise-based filesystem APIs. Stream-uncompression tests use promise-based directory creation and resume entry streams after directory creation succeeds.

Changes: Filesystem Promise Migration

Layer / File(s) Summary
Tar entry processing
lib/tar/file_stream.js, lib/tar/stream.js
Tar file and directory processing uses promise-based stat and directory enumeration. Rejected operations are emitted as stream errors.
Extraction directory setup
lib/utils.js, test/tar/uncompress_stream.test.js, test/tgz/uncompress_stream.test.js, test/zip/uncompress_stream.test.js
Extraction setup creates the destination directory with fs.promises.mkdir. Test handlers create entry directories with fs.promises.mkdir, resume entries after success, and pass rejections to the relevant rejection handler.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: shaoshuai0102

Merge Risk: Minimal · up to 33d6b

No actionable merge-blocking risk was identified in this filesystem promise migration.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "refactor: use filesystem promises to clear lint warnings" matches the main change. The raw summary shows that lib/tar/file_stream.js, lib/tar/stream.js, lib/utils.js, and the uncompress_str…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "refactor: use filesystem promises to clear lint warnings" matches the main change. The raw summary shows that lib/tar/file_stream.js, lib/tar/stream.js, lib/utils.js, and the uncompress_stream tests replace callback-based fs calls (fs.stat, fs.mkdir) with fs.promises equivalents. The PR objectives say this addresses node/prefer-promises/fs warnings. The title names the refactor and its purpose, so it is specific and accurate. It does not mention the small @returns to @return JSDoc fix, which is a minor part of the changeset.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 6 files.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

lib/tar/file_stream.js

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

lib/tar/stream.js

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

lib/utils.js

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

  • 3 others

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

@codecov

codecov Bot commented Nov 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.56%. Comparing base (525e508) to head (33d6be1).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #148      +/-   ##
==========================================
- Coverage   97.57%   97.56%   -0.02%     
==========================================
  Files          19       19              
  Lines        1237     1230       -7     
  Branches      321      317       -4     
==========================================
- Hits         1207     1200       -7     
  Misses         30       30              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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