- 
                Notifications
    You must be signed in to change notification settings 
- Fork 44
feat: replace diskusage with diskusage-ng for improved functionality #2680
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: replace diskusage with diskusage-ng for improved functionality #2680
Conversation
| WalkthroughThe changes replace the "diskusage" dependency with "diskusage-ng" across the project, updating package references and modifying the implementation for retrieving disk usage information. The code now uses the "diskusage-ng" API, switching from an async/await pattern to a callback-based approach wrapped in Promises. No public APIs were altered. Changes
 Sequence Diagram(s)sequenceDiagram
    participant OSInfoFactory as getOperatingSystemInfoFactory
    participant DiskUsageNG as diskusage-ng
    OSInfoFactory->>DiskUsageNG: diskusage(path, callback)
    DiskUsageNG-->>OSInfoFactory: callback(error, usage)
    OSInfoFactory->>OSInfoFactory: Resolve or reject Promise based on callback
Poem
 ✨ Finishing Touches
 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit: 
 SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
 Other keywords and placeholders
 CodeRabbit Configuration File ( | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/dashmate/package.json (1)
73-73: Consider aligning the version range with the package’s semver guarantees
^1.0.4will accept1.x.y, including un-tested minors that may change the native ABI.
To minimise breakage risk for a CLI tool installed globally, you might prefer the conservative~1.0.4, combined with a dependabot rule to bump when vetted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
- .yarn/cache/diskusage-ng-npm-1.0.4-f8171e5032-b219f4197f.zipis excluded by- !**/.yarn/**,- !**/*.zip
- .yarn/cache/diskusage-npm-1.2.0-3d0577ad86-cf37552dd9.zipis excluded by- !**/.yarn/**,- !**/*.zip
- .yarn/cache/nan-npm-2.20.0-5b5be83e88-5f16e4c995.zipis excluded by- !**/.yarn/**,- !**/*.zip
- yarn.lockis excluded by- !**/yarn.lock,- !**/*.lock
📒 Files selected for processing (3)
- .pnp.cjs(2 hunks)
- packages/dashmate/package.json(1 hunks)
- packages/dashmate/src/util/getOperatingSystemInfoFactory.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`packages/dashmate/**`: Dashmate is the node management tool for setting up and managing Dash Platform nodes.
packages/dashmate/**: Dashmate is the node management tool for setting up and managing Dash Platform nodes.
- packages/dashmate/package.json
- packages/dashmate/src/util/getOperatingSystemInfoFactory.js
🧠 Learnings (1)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (7)
packages/dashmate/package.json (2)
73-73: ```shell
#!/usr/bin/env bashSearch for any remaining imports or requires of the old
diskusagepackage.echo "Checking for require('diskusage')..."
rg -n --no-heading "require(['"]diskusage['"])" || echo "✅ no require imports found"echo "Checking for ESM imports from 'diskusage'..."
rg -n --no-heading "import\s+.*\s+from\s+['"]diskusage['"]" || echo "✅ no static import statements found"echo "Checking for dynamic import('diskusage')..."
rg -n --no-heading "import(['"]diskusage['"])" || echo "✅ no dynamic imports found"--- `73-73`: ```shell #!/usr/bin/env bash # Get repository information for [email protected] npm view [email protected] repository.url.pnp.cjs (2)
8647-8647: Dependency entry looks correctThe top-level dependency map now points to
[email protected], matching the PR goal of replacingdiskusage. No issues spotted here.
9176-9181: Please double-check that the legacydiskusagepackage is fully purgedAlthough this hunk correctly adds the new package block, it is easy to overlook stale references to the removed dependency in other parts of the repo (code, configs, or even other sections of
.pnp.cjs). A quick grep will confirm:#!/bin/bash # Search for any residual 'diskusage' (but not 'diskusage-ng') references rg --fixed-strings --line-number --color=always "diskusage" \ --glob '!.pnp.cjs' --glob '!**/yarn.lock' | headExpected: zero results.
packages/dashmate/src/util/getOperatingSystemInfoFactory.js (3)
2-2: LGTM: Import updated correctly.The import statement has been properly updated to use the new
diskusage-ngpackage.
175-183: LGTM: Consistent implementation with the previous usage block.The second usage block correctly follows the same Promise wrapper pattern as the first one, maintaining consistency in error handling and API usage. The platform-specific path selection (
'c:'for Windows,'/'for other platforms) is preserved correctly.
156-164: Verify the API compatibility and Promise wrapper implementation.The implementation correctly wraps the callback-based
diskusage-ngAPI in a Promise to maintain the existing async/await pattern. The error handling follows standard Node.js callback conventions.Please verify that the
diskusage-ngpackage API matches this implementation and returns compatible data structures:#!/bin/bash # Description: Verify diskusage-ng API usage and compatibility # Expected: Find documentation or examples of diskusage-ng callback API usage # Search for diskusage-ng usage examples in node_modules or documentation fd -t f -e md -e json . node_modules/diskusage-ng 2>/dev/null | head -5 | xargs cat 2>/dev/null | grep -A 10 -B 10 "callback\|usage\|example" || echo "Documentation not found in node_modules" # Check if there are any tests that might show the expected API fd -t f -e js . node_modules/diskusage-ng 2>/dev/null | head -3 | xargs grep -l "test\|spec" 2>/dev/null | head -1 | xargs cat 2>/dev/null | head -20 || echo "No test files found"
Issue being fixed or feature implemented
Replaced the
diskusagepackage withdiskusage-ngto enhance disk usage functionality.What was done?
diskusagepackage and all its references.diskusage-ngpackage and updated the relevant imports and usage in the code.How Has This Been Tested?
The changes were tested with existing unit tests that cover disk usage functionality.
Breaking Changes
None
Checklist
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
For repository code-owners and collaborators only
I have assigned this pull request to a milestone
Summary by CodeRabbit