Lightning Polish #161
Open
Lightning Polish #161
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
General cleanup of Lightning integration to mirror recent work on bitcoind stability.
- Increased default retry attempts in the internal
_waitForBitcoindhelper. - Introduced a new script to analyze dependency update frequency and added an npm task.
- Extended CI to install
lightningdon both Ubuntu and macOS runners.
Reviewed Changes
Copilot reviewed 4 out of 7 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| services/bitcoin.js | Bumped default retry count and clarified jsdoc for _waitForBitcoind |
| scripts/analyze-deps.js | New script to report how often dependencies change over history |
| package.json | Added report:noisy script to run analyze-deps.js |
| .github/workflows/test.yaml | Added steps to install Core Lightning on CI agents |
Comments suppressed due to low confidence (3)
scripts/analyze-deps.js:1
- This new script lacks automated tests to verify its functionality; consider adding unit or integration tests to ensure it behaves as expected across various lockfile histories.
#!/usr/bin/env node
package.json:57
- [nitpick] The script name
report:noisyis ambiguous; consider renaming it to something more descriptive likereport:dep-updatesto clarify its purpose.
"report:noisy": "node scripts/analyze-deps.js | tee reports/noisy-deps.txt",
.github/workflows/test.yaml:64
- Using
pip3 install --usermay install packages to~/.local/bin, which might not be in the default PATH in CI; consider installing packages globally (omit--user) or updating the PATH explicitly.
pip3 install --user \
| } | ||
|
|
||
| async _waitForBitcoind (maxAttempts = 10, initialDelay = 2000) { | ||
| async _waitForBitcoind (maxAttempts = 32, initialDelay = 2000) { |
There was a problem hiding this comment.
Consider extracting magic default values like 32 and 2000 into named constants or configuration options to improve readability and ease future adjustments.
Suggested change
| async _waitForBitcoind (maxAttempts = 32, initialDelay = 2000) { | |
| async _waitForBitcoind (maxAttempts = DEFAULT_MAX_ATTEMPTS, initialDelay = DEFAULT_INITIAL_DELAY_MS) { |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/v0.1.0-RC1 #161 +/- ##
======================================================
+ Coverage 60.47% 65.14% +4.67%
======================================================
Files 65 64 -1
Lines 15731 17093 +1362
Branches 0 1 +1
======================================================
+ Hits 9514 11136 +1622
+ Misses 6217 5957 -260 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…e EPIPE error handling - Remove duplicate peer event emission in P2P_SESSION_OPEN case to prevent 'done() called multiple times' error - Add proper EPIPE/ECONNRESET error handling in direct encrypt.write() calls to prevent uncaught exceptions - Tests now pass: 345 passing, 74 pending, 0 failing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
General cleanup of Lightning integration to mirror recent work on
bitcoindstability.