Merged
Conversation
Added a section explaining how Hyper-FS works with Rust and included diagrams for comparison with Node.js.
|
Warning Rate limit exceeded
⌛ 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. 📝 WalkthroughWalkthroughSystematic rebranding from "hyper-fs" to "rush-fs" across package metadata, bindings, docs, tests, benchmarks, and CI. Changes are predominantly textual (names, paths, labels); a Release workflow and optionalDependencies were added to packaging; no public APIs or runtime control flow were materially altered. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
CoderSerio
force-pushed
the
feat/carbon-dev
branch
from
8e8d6ae to
992aea8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
25-123:⚠️ Potential issue | 🟡 MinorDuplicate "How does it works" section — remove the old copy.
Lines 25–73 introduce a newly branded version of the section, but the pre-existing identical section at lines 75–123 was not removed. The published README now renders both copies back-to-back, which will confuse readers.
Delete the block starting at line 75 (
## How does it works) through line 123 (the closing```of the second mermaid diagram).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 25 - 123, Remove the duplicated "## How does it works" block (the second occurrence) and its trailing mermaid diagram: locate the repeated header string "## How does it works" and delete from that header through the closing triple-backtick (```) that ends the second mermaid code fence so only the first, intended section remains.
🧹 Nitpick comments (12)
__test__/stat.spec.ts (1)
29-38: Optional: rename residualhyperStat/hyperLstatvariables to align with the new project name.Internal variable names
hyperStatandhyperLstat(lines 29, 90, 91, 102) weren't updated as part of the rename. No functional impact, but it creates minor inconsistency within the file.♻️ Example rename (applies to all occurrences in this file)
- const hyperStat = statSync('./package.json') + const rushStat = statSync('./package.json')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__test__/stat.spec.ts` around lines 29 - 38, Rename the residual variables hyperStat and hyperLstat to use the project's current prefix instead of "hyper" (e.g., replace hyperStat → <ProjectPrefix>Stat and hyperLstat → <ProjectPrefix>Lstat) so names are consistent; update every occurrence of the symbols hyperStat and hyperLstat in this test file (including the uses in assertions) to the chosen project prefix.__test__/utimes.spec.ts (1)
48-48: Stalehyper-filenames missed by this rename.Lines 48 and 98 still pass
'hyper-utimes.txt'/'hyper-utimes-date.txt'as the filename argument totmpFile(). Since the PR's goal is a complete rebrand, consider updating these torush-equivalents for consistency.♻️ Proposed fix
- const file2 = tmpFile('hyper-utimes.txt') + const file2 = tmpFile('rush-utimes.txt')- const file2 = tmpFile('hyper-utimes-date.txt') + const file2 = tmpFile('rush-utimes-date.txt')Also applies to: 98-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__test__/utimes.spec.ts` at line 48, Two test filenames were not updated during the rebrand: update the calls to tmpFile that still pass 'hyper-utimes.txt' and 'hyper-utimes-date.txt' so they match the new naming convention. Locate the tmpFile(...) invocations in utimes.spec.ts (the ones passing those literal filename strings) and change them to 'rush-utimes.txt' and 'rush-utimes-date.txt' respectively so both occurrences use the consistent "rush-" prefix.benchmark/glob.ts (1)
2-2: StalehyperGlobSyncimport alias not updated.The bench labels now say
rush-fsbut the imported alias on line 2 still readshyperGlobSync, creating a visual inconsistency for anyone reading the benchmark source.♻️ Proposed fix
-import { globSync as hyperGlobSync } from '../index.js' +import { globSync as rushGlobSync } from '../index.js'Then replace the four call sites (
hyperGlobSync→rushGlobSync) on lines 19, 26, 37, and 38.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmark/glob.ts` at line 2, Update the stale import alias and its usages: change the import "hyperGlobSync" to "rushGlobSync" in the import statement (currently importing globSync as hyperGlobSync) and replace all call sites that invoke hyperGlobSync with rushGlobSync (there are four occurrences in this file) so the bench labels and symbol names match.benchmark/mkdir.ts (1)
53-78: Incomplete rename:hyper-single-andhyper-deep-subdirectory fragmentsThe
namedisplay strings (Lines 54, 72) were updated toRush-FS, but the ephemeral subdirectory path fragments on Lines 56 and 74 still use the oldhyper-prefix, leaving the rename inconsistent within the same benchmark.✏️ Suggested fix
- const dir = path.join(tmpDir, `hyper-single-${counter++}`) + const dir = path.join(tmpDir, `rush-single-${counter++}`)- const dir = path.join(tmpDir, `hyper-deep-${counter++}`, 'a', 'b', 'c', 'd') + const dir = path.join(tmpDir, `rush-deep-${counter++}`, 'a', 'b', 'c', 'd')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmark/mkdir.ts` around lines 53 - 78, The displayed benchmark name was changed to "Rush-FS" but the ephemeral directory prefixes still use "hyper-single-" and "hyper-deep-"; update the path fragments created in the two Rush-FS cases (the dir construction inside the runGroup entries where mkdirSync is called) to use matching prefixes (e.g., replace "hyper-single-" and "hyper-deep-" with a consistent "rush-single-" / "rush-deep-" or another "Rush-FS"-aligned prefix) so the benchmark names and generated directories are consistent.CONTRIBUTING.md (1)
321-350: Residualhyper-prefixed variable names in code examplesThe code samples in the "双跑对比测试" section still use
hyperResult(Line 325) andhyperStat/ multiple usages around Line 344–350. These weren't renamed alongside the surrounding prose.✏️ Suggested update
- const hyperResult = readlinkSync(link) - t.is(hyperResult, nodeResult) + const rushResult = readlinkSync(link) + t.is(rushResult, nodeResult)- const hyperStat = statSync('./package.json') - t.is(hyperStat.size, nodeStat.size) - t.is(hyperStat.mode, nodeStat.mode) - t.is(hyperStat.isFile(), nodeStat.isFile()) - t.is(hyperStat.isDirectory(), nodeStat.isDirectory()) + const rushStat = statSync('./package.json') + t.is(rushStat.size, nodeStat.size) + t.is(rushStat.mode, nodeStat.mode) + t.is(rushStat.isFile(), nodeStat.isFile()) + t.is(rushStat.isDirectory(), nodeStat.isDirectory())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` around lines 321 - 350, Update the example variable names that still use the old "hyper" prefix: replace hyperResult with a consistent project-prefixed name (e.g., rushResult) in the readlinkSync example and replace hyperStat with rushStat (and all its usages like rushStat.size, rushStat.mode, rushStat.isFile(), rushStat.isDirectory()) in the statSync double-run comparison so examples match the surrounding prose and the package name (functions referenced: readlinkSync, statSync).__test__/readlink.spec.ts (1)
32-33: VariablehyperResultnot renamed.♻️ Proposed fix
- const hyperResult = readlinkSync(link) - t.is(hyperResult, nodeResult) + const rushResult = readlinkSync(link) + t.is(rushResult, nodeResult)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__test__/readlink.spec.ts` around lines 32 - 33, Rename the local test variable hyperResult to hyperNodeResult in the test that calls readlinkSync so the name clearly distinguishes it from nodeResult; update the variable declaration and the assertion using readlinkSync and nodeResult (references to hyperResult, the call to readlinkSync, and the t.is assertion) accordingly.benchmark/rm.ts (1)
4-4: Import aliashyperRmSyncnot updated to match rebrand.♻️ Proposed fix
-import { rmSync as hyperRmSync } from '../index.js' +import { rmSync as rushRmSync } from '../index.js'Then update the two call sites accordingly:
- { name: 'Rush-FS', fn: (p: string) => hyperRmSync(p, { recursive: true, force: true }) }, + { name: 'Rush-FS', fn: (p: string) => rushRmSync(p, { recursive: true, force: true }) }, { name: 'Rush-FS (4 threads)', - fn: (p: string) => hyperRmSync(p, { recursive: true, force: true, concurrency: 4 }), + fn: (p: string) => rushRmSync(p, { recursive: true, force: true, concurrency: 4 }), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmark/rm.ts` at line 4, The import alias hyperRmSync is outdated after the rebrand—open index.js to find the current exported name for the rmSync function and replace the alias in benchmark/rm.ts with that new export name, then update all call sites in this file that reference hyperRmSync to use the new symbol (keep the imported function name and the calls consistent, e.g., replace hyperRmSync(...) with NewExportedName(...)).__test__/rmdir.spec.ts (1)
44-56: VariableshyperDir/hyperTargetnot renamed in the dual-run test.♻️ Proposed fix
const dir1 = tmpDir('node-rmdir') - const dir2 = tmpDir('hyper-rmdir') + const dir2 = tmpDir('rush-rmdir') const nodeTarget = join(dir1, 'sub') - const hyperTarget = join(dir2, 'sub') + const rushTarget = join(dir2, 'sub') nodeMkdirSync(nodeTarget) - mkdirSync(hyperTarget) + mkdirSync(rushTarget) nodeRmdirSync(nodeTarget) - rmdirSync(hyperTarget) + rmdirSync(rushTarget) - t.is(existsSync(hyperTarget), existsSync(nodeTarget)) + t.is(existsSync(rushTarget), existsSync(nodeTarget))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__test__/rmdir.spec.ts` around lines 44 - 56, The test uses inconsistent variable names for the hyper-run side; rename the hyper-side variables to the dual-run naming convention so they mirror the node-side ones. Specifically, ensure the directory and target vars created with tmpDir and join are named hyperDir and hyperTarget (or whatever the repo-standard hyper* names are) and update usages in tmpDir('hyper-rmdir') assignment, the join(...) call, mkdirSync(hyperTarget), rmdirSync(hyperTarget), and the final existsSync comparison so all references use the same hyper-prefixed identifiers (functions involved: tmpDir, join, nodeMkdirSync, mkdirSync, nodeRmdirSync, rmdirSync, existsSync).__test__/mkdir.spec.ts (1)
89-117: Residualhyperidentifiers and fixture strings not renamed.The dual-run test blocks were missed in the rename sweep:
- Variable names:
hyperDir(lines 91, 94, 96–100),hyperDir/hyperResult(lines 105, 108, 113, 116–117).- Fixture name strings passed to
tmpPath:'hyper-recursive'(line 91) and'hyper-return'(line 105).- The assertion
hyperResult.endsWith('hyper-return')on line 113 still passes today because the suffix is whatever is passed totmpPath, but it will need updating together with the fixture string.♻️ Proposed rename for the dual-run blocks
- const hyperDir = tmpPath('hyper-recursive') + const rushDir = tmpPath('rush-recursive') - nodeFs.mkdirSync(join(nodeDir, 'a', 'b'), { recursive: true }) - mkdirSync(join(hyperDir, 'a', 'b'), { recursive: true }) + nodeFs.mkdirSync(join(nodeDir, 'a', 'b'), { recursive: true }) + mkdirSync(join(rushDir, 'a', 'b'), { recursive: true }) - t.is(existsSync(join(nodeDir, 'a', 'b')), existsSync(join(hyperDir, 'a', 'b'))) - t.is(existsSync(join(nodeDir, 'a')), existsSync(join(hyperDir, 'a'))) + t.is(existsSync(join(nodeDir, 'a', 'b')), existsSync(join(rushDir, 'a', 'b'))) + t.is(existsSync(join(nodeDir, 'a')), existsSync(join(rushDir, 'a'))) - nodeFs.rmSync(hyperDir, { recursive: true }) + nodeFs.rmSync(rushDir, { recursive: true })- const hyperDir = tmpPath('hyper-return') + const rushDir = tmpPath('rush-return') - const hyperResult = mkdirSync(join(hyperDir, 'a', 'b'), { recursive: true }) + const rushResult = mkdirSync(join(rushDir, 'a', 'b'), { recursive: true }) - t.is(typeof hyperResult, typeof nodeResult) - if (nodeResult !== undefined && hyperResult !== undefined) { + t.is(typeof rushResult, typeof nodeResult) + if (nodeResult !== undefined && rushResult !== undefined) { t.true(nodeResult.endsWith('node-return')) - t.true(hyperResult.endsWith('hyper-return')) + t.true(rushResult.endsWith('rush-return')) } - nodeFs.rmSync(hyperDir, { recursive: true }) + nodeFs.rmSync(rushDir, { recursive: true })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__test__/mkdir.spec.ts` around lines 89 - 117, Tests still contain leftover "hyper" identifiers: rename the variables hyperDir and hyperResult and the fixture strings passed to tmpPath ('hyper-recursive' and 'hyper-return') to the project's current naming convention; update the corresponding assertions and cleanup calls that use those variables (references to tmpPath, mkdirSync, nodeFs.mkdirSync, existsSync, rmSync, and the hyperResult.endsWith(...) assertion) so the fixture suffix and variable names match (e.g., replace "hyper-*" with the new prefix) and ensure the endsWith check uses the new fixture string.benchmark/write_file.ts (1)
79-81: Stalehyperreference in append benchmark filename.
append-hyper.txton line 79 was not updated alongside the rest of the file. For consistency with the rebrand it should beappend-rush.txt.♻️ Proposed fix
-const appendTarget2 = path.join(tmpDir, 'append-hyper.txt') +const appendTarget2 = path.join(tmpDir, 'append-rush.txt')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmark/write_file.ts` around lines 79 - 81, The benchmark creates appendTarget2 using path.join(...) with a stale filename 'append-hyper.txt' and then calls fs.writeFileSync on appendTarget2; update the literal in the path.join to 'append-rush.txt' (and ensure any related variable names or subsequent references to appendTarget2 remain correct) so the filename matches the rebrand used elsewhere.__test__/unlink.spec.ts (1)
52-59: Stalehypervariable name and filename in dual-run test.
hyperFile/hyper.txton lines 52–59 were not updated as part of the rebrand.♻️ Proposed fix
- const hyperFile = join(dir, 'hyper.txt') + const rushFile = join(dir, 'rush.txt') writeFileSync(nodeFile, 'x') - writeFileSync(hyperFile, 'x') + writeFileSync(rushFile, 'x') nodeFs.unlinkSync(nodeFile) - unlinkSync(hyperFile) + unlinkSync(rushFile) - t.is(existsSync(hyperFile), existsSync(nodeFile)) + t.is(existsSync(rushFile), existsSync(nodeFile))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__test__/unlink.spec.ts` around lines 52 - 59, The test still uses the old hyper naming: rename the hyper-related variable and file from hyperFile/hyper.txt to the new name (e.g., altFile/alt.txt) and update all references in this block (writeFileSync(hyperFile, ...), unlinkSync(hyperFile), and existsSync(hyperFile)) so they match nodeFile equivalents; ensure the calls to nodeFs.unlinkSync(nodeFile), unlinkSync(<newName>), writeFileSync(nodeFile, ...), writeFileSync(<newName>, ...), and the final t.is(existsSync(<newName>), existsSync(nodeFile)) are all using the updated identifier and filename consistently.__test__/cp.spec.ts (1)
162-168: Stalehypervariable names and file paths in dual-run tests.Lines 162–167 and 178–191 still use
hyperDest/hyper-dest(both as variable names and as the actual file/directory paths on disk). These were not updated as part of the rebrand.♻️ Proposed fix (first dual-run test)
- const hyperDest = join(dir, 'hyper-dest.txt') + const rushDest = join(dir, 'rush-dest.txt') nodeFs.cpSync(src, nodeDest) - cpSync(src, hyperDest) + cpSync(src, rushDest) - t.is(readFileSync(hyperDest, 'utf8'), readFileSync(nodeDest, 'utf8')) + t.is(readFileSync(rushDest, 'utf8'), readFileSync(nodeDest, 'utf8'))♻️ Proposed fix (second dual-run test)
- const hyperDest = join(dir, 'hyper-dest') + const rushDest = join(dir, 'rush-dest') nodeFs.cpSync(src, nodeDest, { recursive: true }) - cpSync(src, hyperDest, { recursive: true }) + cpSync(src, rushDest, { recursive: true }) - const hyperFiles = readdirSync(hyperDest, { recursive: true }) as string[] + const rushFiles = readdirSync(rushDest, { recursive: true }) as string[] - t.deepEqual(hyperFiles.sort(), nodeFiles.sort()) + t.deepEqual(rushFiles.sort(), nodeFiles.sort()) - t.is(readFileSync(join(hyperDest, 'root.txt'), 'utf8'), ...) + t.is(readFileSync(join(rushDest, 'root.txt'), 'utf8'), ...) - t.is(readFileSync(join(hyperDest, 'sub', 'child.txt'), 'utf8'), ...) + t.is(readFileSync(join(rushDest, 'sub', 'child.txt'), 'utf8'), ...)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__test__/cp.spec.ts` around lines 162 - 168, Tests still use legacy names hyperDest and files named 'hyper-dest*'; rename the variable and on-disk path to the new naming used elsewhere (e.g., replace hyperDest with altDest or cpDest and 'hyper-dest.txt' with 'alt-dest.txt' or 'cp-dest.txt') in both dual-run tests (the occurrences around cpSync vs nodeFs.cpSync and the second dual-run block later). Update all references (variable declaration, path construction, and the t.is readFileSync assertions) so the variable and filename are consistent with the rebrand, ensuring both runs compare the same new file path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 8: The package.json "repository.url" entry currently points to
"git+https://github.com/rush-fs/rush-fs.git" which doesn't match the actual
GitHub repo; update the repository.url field in package.json to the correct
repository URL (for example "git+https://github.com/CoderSerio/rush-fs.git" if
the project remains under CoderSerio) so the "repository" object accurately
reflects the real repository; locate and update the "repository.url" key in
package.json to the agreed canonical URL.
- Around line 117-122: The optionalDependencies block in package.json lists four
platform-specific packages (rush-fs-win32-x64-msvc, rush-fs-darwin-x64,
rush-fs-linux-x64-gnu, rush-fs-darwin-arm64) that are not yet published to npm;
update your release workflow so the prepublishOnly step (or your release script)
publishes those four platform packages first, verifies each package exists on
npm (e.g., by querying the npm registry or using npm view), and only then
publishes the main rush-fs package to ensure native binaries are available to
consumers.
---
Outside diff comments:
In `@README.md`:
- Around line 25-123: Remove the duplicated "## How does it works" block (the
second occurrence) and its trailing mermaid diagram: locate the repeated header
string "## How does it works" and delete from that header through the closing
triple-backtick (```) that ends the second mermaid code fence so only the first,
intended section remains.
---
Nitpick comments:
In `@__test__/cp.spec.ts`:
- Around line 162-168: Tests still use legacy names hyperDest and files named
'hyper-dest*'; rename the variable and on-disk path to the new naming used
elsewhere (e.g., replace hyperDest with altDest or cpDest and 'hyper-dest.txt'
with 'alt-dest.txt' or 'cp-dest.txt') in both dual-run tests (the occurrences
around cpSync vs nodeFs.cpSync and the second dual-run block later). Update all
references (variable declaration, path construction, and the t.is readFileSync
assertions) so the variable and filename are consistent with the rebrand,
ensuring both runs compare the same new file path.
In `@__test__/mkdir.spec.ts`:
- Around line 89-117: Tests still contain leftover "hyper" identifiers: rename
the variables hyperDir and hyperResult and the fixture strings passed to tmpPath
('hyper-recursive' and 'hyper-return') to the project's current naming
convention; update the corresponding assertions and cleanup calls that use those
variables (references to tmpPath, mkdirSync, nodeFs.mkdirSync, existsSync,
rmSync, and the hyperResult.endsWith(...) assertion) so the fixture suffix and
variable names match (e.g., replace "hyper-*" with the new prefix) and ensure
the endsWith check uses the new fixture string.
In `@__test__/readlink.spec.ts`:
- Around line 32-33: Rename the local test variable hyperResult to
hyperNodeResult in the test that calls readlinkSync so the name clearly
distinguishes it from nodeResult; update the variable declaration and the
assertion using readlinkSync and nodeResult (references to hyperResult, the call
to readlinkSync, and the t.is assertion) accordingly.
In `@__test__/rmdir.spec.ts`:
- Around line 44-56: The test uses inconsistent variable names for the hyper-run
side; rename the hyper-side variables to the dual-run naming convention so they
mirror the node-side ones. Specifically, ensure the directory and target vars
created with tmpDir and join are named hyperDir and hyperTarget (or whatever the
repo-standard hyper* names are) and update usages in tmpDir('hyper-rmdir')
assignment, the join(...) call, mkdirSync(hyperTarget), rmdirSync(hyperTarget),
and the final existsSync comparison so all references use the same
hyper-prefixed identifiers (functions involved: tmpDir, join, nodeMkdirSync,
mkdirSync, nodeRmdirSync, rmdirSync, existsSync).
In `@__test__/stat.spec.ts`:
- Around line 29-38: Rename the residual variables hyperStat and hyperLstat to
use the project's current prefix instead of "hyper" (e.g., replace hyperStat →
<ProjectPrefix>Stat and hyperLstat → <ProjectPrefix>Lstat) so names are
consistent; update every occurrence of the symbols hyperStat and hyperLstat in
this test file (including the uses in assertions) to the chosen project prefix.
In `@__test__/unlink.spec.ts`:
- Around line 52-59: The test still uses the old hyper naming: rename the
hyper-related variable and file from hyperFile/hyper.txt to the new name (e.g.,
altFile/alt.txt) and update all references in this block
(writeFileSync(hyperFile, ...), unlinkSync(hyperFile), and
existsSync(hyperFile)) so they match nodeFile equivalents; ensure the calls to
nodeFs.unlinkSync(nodeFile), unlinkSync(<newName>), writeFileSync(nodeFile,
...), writeFileSync(<newName>, ...), and the final t.is(existsSync(<newName>),
existsSync(nodeFile)) are all using the updated identifier and filename
consistently.
In `@__test__/utimes.spec.ts`:
- Line 48: Two test filenames were not updated during the rebrand: update the
calls to tmpFile that still pass 'hyper-utimes.txt' and 'hyper-utimes-date.txt'
so they match the new naming convention. Locate the tmpFile(...) invocations in
utimes.spec.ts (the ones passing those literal filename strings) and change them
to 'rush-utimes.txt' and 'rush-utimes-date.txt' respectively so both occurrences
use the consistent "rush-" prefix.
In `@benchmark/glob.ts`:
- Line 2: Update the stale import alias and its usages: change the import
"hyperGlobSync" to "rushGlobSync" in the import statement (currently importing
globSync as hyperGlobSync) and replace all call sites that invoke hyperGlobSync
with rushGlobSync (there are four occurrences in this file) so the bench labels
and symbol names match.
In `@benchmark/mkdir.ts`:
- Around line 53-78: The displayed benchmark name was changed to "Rush-FS" but
the ephemeral directory prefixes still use "hyper-single-" and "hyper-deep-";
update the path fragments created in the two Rush-FS cases (the dir construction
inside the runGroup entries where mkdirSync is called) to use matching prefixes
(e.g., replace "hyper-single-" and "hyper-deep-" with a consistent
"rush-single-" / "rush-deep-" or another "Rush-FS"-aligned prefix) so the
benchmark names and generated directories are consistent.
In `@benchmark/rm.ts`:
- Line 4: The import alias hyperRmSync is outdated after the rebrand—open
index.js to find the current exported name for the rmSync function and replace
the alias in benchmark/rm.ts with that new export name, then update all call
sites in this file that reference hyperRmSync to use the new symbol (keep the
imported function name and the calls consistent, e.g., replace hyperRmSync(...)
with NewExportedName(...)).
In `@benchmark/write_file.ts`:
- Around line 79-81: The benchmark creates appendTarget2 using path.join(...)
with a stale filename 'append-hyper.txt' and then calls fs.writeFileSync on
appendTarget2; update the literal in the path.join to 'append-rush.txt' (and
ensure any related variable names or subsequent references to appendTarget2
remain correct) so the filename matches the rebrand used elsewhere.
In `@CONTRIBUTING.md`:
- Around line 321-350: Update the example variable names that still use the old
"hyper" prefix: replace hyperResult with a consistent project-prefixed name
(e.g., rushResult) in the readlinkSync example and replace hyperStat with
rushStat (and all its usages like rushStat.size, rushStat.mode,
rushStat.isFile(), rushStat.isDirectory()) in the statSync double-run comparison
so examples match the surrounding prose and the package name (functions
referenced: readlinkSync, statSync).
package.json
Outdated
Comment on lines
+117
to
+122
| "optionalDependencies": { | ||
| "rush-fs-win32-x64-msvc": "0.0.1", | ||
| "rush-fs-darwin-x64": "0.0.1", | ||
| "rush-fs-linux-x64-gnu": "0.0.1", | ||
| "rush-fs-darwin-arm64": "0.0.1" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the platform packages already exist on the npm registry
for pkg in rush-fs-win32-x64-msvc rush-fs-darwin-x64 rush-fs-linux-x64-gnu rush-fs-darwin-arm64; do
echo "=== $pkg ==="
curl -sf "https://registry.npmjs.org/$pkg/0.0.1" | jq '{name: .name, version: .version}' 2>/dev/null || echo "NOT FOUND"
doneRepository: CoderSerio/hyper-fs
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Check whether the platform packages already exist on the npm registry
for pkg in rush-fs-win32-x64-msvc rush-fs-darwin-x64 rush-fs-linux-x64-gnu rush-fs-darwin-arm64; do
echo "=== Checking $pkg ==="
HTTP_CODE=$(curl -s -o /tmp/pkg_response.json -w "%{http_code}" "https://registry.npmjs.org/$pkg/0.0.1")
echo "HTTP Status: $HTTP_CODE"
if [ "$HTTP_CODE" = "200" ]; then
echo "FOUND"
cat /tmp/pkg_response.json | head -50
else
echo "NOT FOUND (HTTP $HTTP_CODE)"
fi
echo ""
doneRepository: CoderSerio/hyper-fs
Length of output: 371
🏁 Script executed:
cat -n package.json | head -150Repository: CoderSerio/hyper-fs
Length of output: 3823
Ensure all four platform-specific packages are published to npm before releasing the main package — they are currently missing.
The optionalDependencies structure and versions are correct for NAPI-RS, and the targets align perfectly with your napi.targets. However, verification shows that none of the four platform packages exist on npm yet:
rush-fs-win32-x64-msvc@0.0.1— NOT FOUNDrush-fs-darwin-x64@0.0.1— NOT FOUNDrush-fs-linux-x64-gnu@0.0.1— NOT FOUNDrush-fs-darwin-arm64@0.0.1— NOT FOUND
If the main rush-fs package is released before these are published, consumers will install the main package but fail to obtain native binaries. Although the optional dependencies won't hard-fail the install, the package will be unusable without them. Your prepublishOnly script should handle this, but you must execute the full release workflow in the correct order: publish all four platform packages first, then publish the main package.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 117 - 122, The optionalDependencies block in
package.json lists four platform-specific packages (rush-fs-win32-x64-msvc,
rush-fs-darwin-x64, rush-fs-linux-x64-gnu, rush-fs-darwin-arm64) that are not
yet published to npm; update your release workflow so the prepublishOnly step
(or your release script) publishes those four platform packages first, verifies
each package exists on npm (e.g., by querying the npm registry or using npm
view), and only then publishes the main rush-fs package to ensure native
binaries are available to consumers.
6ca9ea5 to
2a389ba
Compare
CoderSerio
force-pushed
the
feat/carbon-dev
branch
from
d787088 to
9a510b8
Compare
CoderSerio
force-pushed
the
feat/carbon-dev
branch
from
9a510b8 to
1736557
Compare
CoderSerio
force-pushed
the
feat/carbon-dev
branch
2 times, most recently
from
b81c40f to
c9b58db
Compare
CoderSerio
force-pushed
the
feat/carbon-dev
branch
from
c9b58db to
05a6a2f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@index.js`:
- Around line 80-82: The hardcoded expected version string in the generated
index.js causes the bindingPackageVersion check (the if that throws Error when
bindingPackageVersion !== '0.0.2') to fail for v0.0.3; regenerate the JS binding
so the expected version matches package.json by running the native build (e.g.
run pnpm build or napi build --platform) so index.js is rebuilt with '0.0.3', or
as a temporary stopgap replace the stale '0.0.2' literal with '0.0.3' so the
throw in the bindingPackageVersion check and the Error message align with the
package version (this affects the code path that checks
NAPI_RS_ENFORCE_VERSION_CHECK).
In `@package.json`:
- Around line 117-122: Remove the optionalDependencies block from the committed
package.json (the "optionalDependencies" object containing rush-fs-* entries) so
the lockfile and CI (--frozen-lockfile) match the repo as documented; delete
that entire optionalDependencies section from package.json and ensure any
local/release-only additions are applied only in the release commit or local
environment per the README's "Before publishing" guidance.
In `@README.md`:
- Around line 75-123: The README contains a duplicated "How does it works"
section (the heading and the two mermaid diagram blocks are repeated); remove
the entire duplicate block so only one section remains and correct the surviving
heading text from "How does it works" to "How does it work" (search for the
heading string "How does it works" and delete the duplicated mermaid/code block
while updating the remaining heading to the grammatically correct form).
---
Duplicate comments:
In `@package.json`:
- Line 8: The package.json "url" field currently points to the wrong org; update
the "url" value in package.json (the "url" property under the repository
metadata) to the correct git+https repository location for this project (use the
actual GitHub org/name for this repo), ensuring the string uses the git+https
format and matches the canonical repository URL used by the project; also check
and update any other repository-related fields (e.g., "repository.url") to the
same correct value.
package.json
Outdated
Comment on lines
+117
to
+122
| "optionalDependencies": { | ||
| "rush-fs-win32-x64-msvc": "0.0.3", | ||
| "rush-fs-darwin-x64": "0.0.3", | ||
| "rush-fs-linux-x64-gnu": "0.0.3", | ||
| "rush-fs-darwin-arm64": "0.0.3" | ||
| } |
There was a problem hiding this comment.
optionalDependencies in the repo directly causes the CI lockfile failure and contradicts the README.
The README's "Publishing (Maintainers Only)" section explicitly states:
"In this repo,
optionalDependenciesare omitted so CI can usepnpm install --frozen-lockfile(the platform packages are not published until release). Before publishing, add them back."
Including these in the committed package.json breaks pnpm install --frozen-lockfile (confirmed by the pipeline failure: "Lockfile specifiers do not match package.json"). The optionalDependencies block must be removed from this file; it should only exist locally / in the release commit, never committed to the repo.
🔧 Fix: remove optionalDependencies from the committed package.json
- "optionalDependencies": {
- "rush-fs-win32-x64-msvc": "0.0.3",
- "rush-fs-darwin-x64": "0.0.3",
- "rush-fs-linux-x64-gnu": "0.0.3",
- "rush-fs-darwin-arm64": "0.0.3"
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "optionalDependencies": { | |
| "rush-fs-win32-x64-msvc": "0.0.3", | |
| "rush-fs-darwin-x64": "0.0.3", | |
| "rush-fs-linux-x64-gnu": "0.0.3", | |
| "rush-fs-darwin-arm64": "0.0.3" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 117 - 122, Remove the optionalDependencies block
from the committed package.json (the "optionalDependencies" object containing
rush-fs-* entries) so the lockfile and CI (--frozen-lockfile) match the repo as
documented; delete that entire optionalDependencies section from package.json
and ensure any local/release-only additions are applied only in the release
commit or local environment per the README's "Before publishing" guidance.
5b2299c to
a7b0c50
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
.github/workflows/Release.yml (5)
82-84: Dead step:matrix.settings.setupis never defined.No matrix entry defines a
setupproperty, so this step is unreachable dead code. Theifguard prevents a runtime failure, but actionlint correctly flags the undefined property.Remove the step or add a
setupproperty to matrix entries that need it.Proposed fix
- - name: Setup toolchain - run: ${{ matrix.settings.setup }} - if: ${{ matrix.settings.setup }} - shell: bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/Release.yml around lines 82 - 84, The "Setup toolchain" job step is dead because matrix.settings.setup is never defined; either remove the step named "Setup toolchain" or add a setup property to the matrix entries that require it. Locate the workflow step with the name "Setup toolchain" and either delete that step block (and any dependent steps) or modify the matrix configuration to include a settings.setup boolean/value for the relevant matrix entries so the if: ${{ matrix.settings.setup }} guard becomes valid.
60-70: Cargo cache key should include aCargo.lockhash.The current cache key (
target-cargo-host) never changes when Rust dependencies are updated, so stale crates can persist across runs. IncludinghashFiles('**/Cargo.lock')in the key is the standard practice to invalidate the cache on dependency changes.Proposed fix
- key: ${{ matrix.settings.target }}-cargo-${{ matrix.settings.host }} + key: ${{ matrix.settings.target }}-cargo-${{ matrix.settings.host }}-${{ hashFiles('**/Cargo.lock') }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/Release.yml around lines 60 - 70, The cache key for the "Cache cargo" step currently uses only matrix.settings.target and matrix.settings.host, so dependency updates don't bust the cache; update the key in the actions/cache step (the step named "Cache cargo") to append the Cargo.lock hash by using hashFiles('**/Cargo.lock') (e.g. include -${{ hashFiles('**/Cargo.lock') }}) so the cache is invalidated when Cargo.lock changes while preserving the existing matrix.settings.target and matrix.settings.host parts.
71-81: Unreachable musl/Zig steps — no musl target in the matrix.The Zig and
cargo-zigbuildsteps are gated oncontains(matrix.settings.target, 'musl'), but none of the four matrix targets containmusl. These steps will never execute. If musl support is planned for later, consider adding a comment; otherwise, remove the dead steps to reduce noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/Release.yml around lines 71 - 81, The workflow contains dead steps guarded by the condition contains(matrix.settings.target, 'musl') — namely the mlugg/setup-zig@v2 step and the "Install cargo-zigbuild" step — but the matrix.targets do not include any musl entries; either remove these two steps (mlugg/setup-zig@v2 and the step named "Install cargo-zigbuild") to eliminate dead code, or add a musl target to the build matrix (the matrix.settings.target entries) or a TODO/comment above the condition indicating planned musl support so the guard makes sense; update the condition or matrix accordingly and ensure the referenced condition contains(matrix.settings.target, 'musl') is consistent with the matrix values.
130-142: Hardcoded optional dependency names — keep in sync with the matrix.The four package names here must match what
napi-rsgenerates innpm/and must stay aligned with the build matrix. If a target is added or removed from the matrix, this block must be updated manually. Consider adding a comment to that effect, or deriving the list from the downloaded artifacts directory to avoid drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/Release.yml around lines 130 - 142, The optional dependency names are hardcoded in the node -e block (the p.optionalDependencies assignment and 'rush-fs-*-*' entries) and can drift from the build matrix; change the script to derive the list dynamically (e.g., scan the downloaded artifacts/npm directory or parse the build matrix/environment) and populate p.optionalDependencies from those filenames instead of hardcoding, or at minimum add a clear comment above the node -e block noting that the four 'rush-fs-...' entries must be kept in sync with the matrix and listing where to update them.
43-47: Caching~/.rustupcan cause issues across runner image updates.GitHub-hosted runner images ship a pre-installed Rust toolchain under
~/.rustup. Caching this directory means a stale toolchain could persist even after GitHub updates the runner image. Thedtolnay/rust-toolchainaction on Line 56 already handles toolchain installation and is fast. Consider dropping this cache step or using arestore-keysfallback to reduce the chance of a corrupted/stale toolchain cache.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/Release.yml around lines 43 - 47, The "Cache Rust toolchain" step that caches path ~/.rustup should be removed or changed to avoid persisting a stale toolchain; update the workflow to either delete the entire "Cache Rust toolchain" step (the step with name "Cache Rust toolchain" and key rustup-${{ matrix.settings.host }}-${{ matrix.settings.target }}-stable) or modify it to add a safe restore-keys fallback and narrower paths, but prefer removing it since the dtolnay/rust-toolchain action already manages installation quickly—adjust the Release.yml to eliminate the problematic cache entry or implement a restore-keys strategy to avoid preserving a stale runner-installed toolchain.README.zh-CN.md (1)
431-431: Commit-message-based release trigger is fragile — considerworkflow_dispatchor a git tag.Any push to
mainwhose commit message is exactly a semver string (e.g.,0.0.3) will silently trigger a publish. Accidental releases are easy to cause (e.g., a bump commit whose message is just the version). Standard alternatives that make intent explicit:
workflow_dispatch: add a manual trigger with aversioninput, so the publisher actively invokes the release workflow from the Actions tab.- Git tag push (
v0.0.3): trigger the workflow onpush: tags: ['v*'], which is the most common convention and integrates cleanly with GitHub Releases.Documenting the current pattern as-is is fine for now, but flagging the risk so maintainers are aware.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.zh-CN.md` at line 431, The README currently instructs to trigger releases by pushing to main with a commit message that is exactly the semver, which is fragile; update the documentation and CI trigger recommendation to use an explicit mechanism instead: replace or augment the sentence mentioning "提交并推送到 `main`，且该次提交的 commit message 仅为版本号" with guidance to use either a manual GitHub Actions trigger (`workflow_dispatch` with a required `version` input) or a git-tag-based trigger (push of tags like `v0.0.3` via `push: tags: ['v*']`), and mention that the workflow should be wired to the publisher job (instead of matching commit-message strings) so accidental releases are prevented.index.js (1)
539-550: Minor: second WASI load error is silently dropped whenNAPI_RS_FORCE_WASIis not set.When native binding resolution fails and both WASI fallbacks also fail, the
catchblock at line 544 is guarded byif (process.env.NAPI_RS_FORCE_WASI). IfNAPI_RS_FORCE_WASIis unset, therush-fs-wasm32-wasiload error is never added toloadErrors, making the final "Cannot find native binding" error at line 563 miss it in its cause chain.Since this is NAPI-RS-generated code the pattern is likely intentional — just worth confirming the upstream template behaves this way intentionally (i.e., WASI errors are only surfaced when WASI is explicitly forced).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.js` around lines 539 - 550, The WASI require catch currently only records the error into loadErrors when process.env.NAPI_RS_FORCE_WASI is truthy, causing the rush-fs-wasm32-wasi failure to be omitted when NAPI_RS_FORCE_WASI is not set; update the catch in the block that loads 'rush-fs-wasm32-wasi' so that the thrown err is always pushed into loadErrors (preserving the existing wasiBindingError assignment/causal chaining only when NAPI_RS_FORCE_WASI is true) — modify the catch handling around nativeBinding/wasiBinding/wasiBindingError/loadErrors so loadErrors.unshift(err) (or equivalent push) always runs, while leaving the conditional logic that sets wasiBindingError.cause intact for the forced-WASI path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 492: Edit the README heading that currently reads "### How to verify the
Mac build (方式 B 第 2 步后)" and remove the Chinese draft annotation so it becomes
"### How to verify the Mac build"; locate the heading string "How to verify the
Mac build (方式 B 第 2 步后)" and replace it with the clean English-only heading "How
to verify the Mac build" to avoid confusing readers.
- Line 83: Replace the full-width Unicode colon in the legend entry string
"✨：New feature from rush-fs" with the ASCII colon so it matches the other
entries; update the text to "✨: New feature from rush-fs" (ensure there's a
regular colon U+003A and a space after it) and save the README so the three
legend lines use consistent ASCII punctuation.
In `@README.zh-CN.md`:
- Line 429: Update the README.zh-CN.md instruction about NPM_TOKEN to prefer an
npm Granular Access Token instead of a Classic Token: replace the sentence that
currently says "NPM_TOKEN（npm 账号生成的 Classic Token，需允许发布）" with guidance to
create a Granular Access Token scoped to the specific package (e.g., rush-fs)
with publish rights and store it as NPM_TOKEN in GitHub Actions Secrets; keep
the note that the token must allow package publish but explicitly recommend
granular scope over account-scoped Classic Tokens to limit blast radius if
leaked.
---
Duplicate comments:
In `@README.md`:
- Line 25: The heading text "How does it works" is grammatically incorrect;
change that heading to "How does it work" by updating the heading string (look
for the line containing the heading "How does it works" in README.md) so the
surviving section uses the correct grammar.
---
Nitpick comments:
In @.github/workflows/Release.yml:
- Around line 82-84: The "Setup toolchain" job step is dead because
matrix.settings.setup is never defined; either remove the step named "Setup
toolchain" or add a setup property to the matrix entries that require it. Locate
the workflow step with the name "Setup toolchain" and either delete that step
block (and any dependent steps) or modify the matrix configuration to include a
settings.setup boolean/value for the relevant matrix entries so the if: ${{
matrix.settings.setup }} guard becomes valid.
- Around line 60-70: The cache key for the "Cache cargo" step currently uses
only matrix.settings.target and matrix.settings.host, so dependency updates
don't bust the cache; update the key in the actions/cache step (the step named
"Cache cargo") to append the Cargo.lock hash by using hashFiles('**/Cargo.lock')
(e.g. include -${{ hashFiles('**/Cargo.lock') }}) so the cache is invalidated
when Cargo.lock changes while preserving the existing matrix.settings.target and
matrix.settings.host parts.
- Around line 71-81: The workflow contains dead steps guarded by the condition
contains(matrix.settings.target, 'musl') — namely the mlugg/setup-zig@v2 step
and the "Install cargo-zigbuild" step — but the matrix.targets do not include
any musl entries; either remove these two steps (mlugg/setup-zig@v2 and the step
named "Install cargo-zigbuild") to eliminate dead code, or add a musl target to
the build matrix (the matrix.settings.target entries) or a TODO/comment above
the condition indicating planned musl support so the guard makes sense; update
the condition or matrix accordingly and ensure the referenced condition
contains(matrix.settings.target, 'musl') is consistent with the matrix values.
- Around line 130-142: The optional dependency names are hardcoded in the node
-e block (the p.optionalDependencies assignment and 'rush-fs-*-*' entries) and
can drift from the build matrix; change the script to derive the list
dynamically (e.g., scan the downloaded artifacts/npm directory or parse the
build matrix/environment) and populate p.optionalDependencies from those
filenames instead of hardcoding, or at minimum add a clear comment above the
node -e block noting that the four 'rush-fs-...' entries must be kept in sync
with the matrix and listing where to update them.
- Around line 43-47: The "Cache Rust toolchain" step that caches path ~/.rustup
should be removed or changed to avoid persisting a stale toolchain; update the
workflow to either delete the entire "Cache Rust toolchain" step (the step with
name "Cache Rust toolchain" and key rustup-${{ matrix.settings.host }}-${{
matrix.settings.target }}-stable) or modify it to add a safe restore-keys
fallback and narrower paths, but prefer removing it since the
dtolnay/rust-toolchain action already manages installation quickly—adjust the
Release.yml to eliminate the problematic cache entry or implement a restore-keys
strategy to avoid preserving a stale runner-installed toolchain.
In `@index.js`:
- Around line 539-550: The WASI require catch currently only records the error
into loadErrors when process.env.NAPI_RS_FORCE_WASI is truthy, causing the
rush-fs-wasm32-wasi failure to be omitted when NAPI_RS_FORCE_WASI is not set;
update the catch in the block that loads 'rush-fs-wasm32-wasi' so that the
thrown err is always pushed into loadErrors (preserving the existing
wasiBindingError assignment/causal chaining only when NAPI_RS_FORCE_WASI is
true) — modify the catch handling around
nativeBinding/wasiBinding/wasiBindingError/loadErrors so loadErrors.unshift(err)
(or equivalent push) always runs, while leaving the conditional logic that sets
wasiBindingError.cause intact for the forced-WASI path.
In `@README.zh-CN.md`:
- Line 431: The README currently instructs to trigger releases by pushing to
main with a commit message that is exactly the semver, which is fragile; update
the documentation and CI trigger recommendation to use an explicit mechanism
instead: replace or augment the sentence mentioning "提交并推送到 `main`，且该次提交的 commit
message 仅为版本号" with guidance to use either a manual GitHub Actions trigger
(`workflow_dispatch` with a required `version` input) or a git-tag-based trigger
(push of tags like `v0.0.3` via `push: tags: ['v*']`), and mention that the
workflow should be wired to the publisher job (instead of matching
commit-message strings) so accidental releases are prevented.
| > - ✅: Fully Supported | ||
| > - 🚧: Partially Supported / WIP | ||
| > - ✨：New feature from hyper-fs | ||
| > - ✨：New feature from rush-fs |
There was a problem hiding this comment.
Full-width colon ： on line 83 — inconsistent with the other legend entries
All three sibling entries use an ASCII colon (:); this entry uses a full-width Unicode colon (：, U+FF1A).
✏️ Proposed fix
-> - ✨：New feature from rush-fs
+> - ✨: New feature from rush-fs📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| > - ✨：New feature from rush-fs | |
| > - ✨: New feature from rush-fs |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 83, Replace the full-width Unicode colon in the legend
entry string "✨：New feature from rush-fs" with the ASCII colon so it matches the
other entries; update the text to "✨: New feature from rush-fs" (ensure there's
a regular colon U+003A and a space after it) and save the README so the three
legend lines use consistent ASCII punctuation.
|
|
||
| If any platform publish fails, fix it and re-run `pnpm prepublishOnly` before retrying `pnpm publish` so consumers never receive a release referring to missing optional dependencies. | ||
|
|
||
| ### How to verify the Mac build (方式 B 第 2 步后) |
There was a problem hiding this comment.
Remove the Chinese draft annotation from the section heading
(方式 B 第 2 步后) ("Method B, after step 2") is a personal draft note that was left in the heading. It will confuse English-speaking maintainers reading the published README.
✏️ Proposed fix
-### How to verify the Mac build (方式 B 第 2 步后)
+### How to verify the Mac build📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### How to verify the Mac build (方式 B 第 2 步后) | |
| ### How to verify the Mac build |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 492, Edit the README heading that currently reads "### How
to verify the Mac build (方式 B 第 2 步后)" and remove the Chinese draft annotation
so it becomes "### How to verify the Mac build"; locate the heading string "How
to verify the Mac build (方式 B 第 2 步后)" and replace it with the clean
English-only heading "How to verify the Mac build" to avoid confusing readers.
|
|
||
| CI 已在多平台（macOS x64/arm64、Windows、Linux）构建并测试，通过后可由同一 workflow 发布到 npm。 | ||
|
|
||
| 1. 在仓库 **Settings → Secrets and variables → Actions** 里添加 **NPM_TOKEN**（npm 账号生成的 Classic Token，需允许发布）。 |
There was a problem hiding this comment.
Prefer a Granular Access Token over a Classic Token for NPM_TOKEN.
npm Classic Tokens are account-scoped and grant broad publish rights across all packages. npm Granular Access Tokens (available since late 2022) let you restrict the token to rush-fs specifically, limiting blast radius if the secret is leaked.
📝 Suggested wording update
-1. 在仓库 **Settings → Secrets and variables → Actions** 里添加 **NPM_TOKEN**（npm 账号生成的 Classic Token，需允许发布）。
+1. 在仓库 **Settings → Secrets and variables → Actions** 里添加 **NPM_TOKEN**（推荐使用 npm 账号生成的 **Granular Access Token**，将权限限定为仅 `rush-fs` 包的 publish；如需兼容旧工具也可使用 Classic Automation Token）。📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. 在仓库 **Settings → Secrets and variables → Actions** 里添加 **NPM_TOKEN**（npm 账号生成的 Classic Token，需允许发布）。 | |
| 1. 在仓库 **Settings → Secrets and variables → Actions** 里添加 **NPM_TOKEN**（推荐使用 npm 账号生成的 **Granular Access Token**，将权限限定为仅 `rush-fs` 包的 publish；如需兼容旧工具也可使用 Classic Automation Token）。 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.zh-CN.md` at line 429, Update the README.zh-CN.md instruction about
NPM_TOKEN to prefer an npm Granular Access Token instead of a Classic Token:
replace the sentence that currently says "NPM_TOKEN（npm 账号生成的 Classic
Token，需允许发布）" with guidance to create a Granular Access Token scoped to the
specific package (e.g., rush-fs) with publish rights and store it as NPM_TOKEN
in GitHub Actions Secrets; keep the note that the token must allow package
publish but explicitly recommend granular scope over account-scoped Classic
Tokens to limit blast radius if leaked.
a7b0c50 to
429faea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Chores
Documentation
Tests & Benchmarks
CI