Skip to content

fix: use bitgesell-core debug namespace - #2

Open
TUPM96 wants to merge 1 commit into
BitgesellOfficial:mainfrom
TUPM96:codex/default-logger-namespace
Open

fix: use bitgesell-core debug namespace#2
TUPM96 wants to merge 1 commit into
BitgesellOfficial:mainfrom
TUPM96:codex/default-logger-namespace

Conversation

@TUPM96

@TUPM96 TUPM96 commented May 25, 2026

Copy link
Copy Markdown

Summary

  • change the default debug logger namespace from bitcoin-core to bitgesell-core
  • add a focused regression test that stubs debugnyan and verifies the default namespace

Why

The package name, README logging examples, and Bitgesell fork branding use bitgesell-core, but the default constructor still initialized debugnyan('bitcoin-core'). That makes users enabling DEBUG logs with the documented bitgesell-core namespace miss default client logs.

Validation

  • npm install --ignore-scripts (regular npm install currently triggers the existing dependencies lifecycle script and fails without docker-compose config; this is already covered separately by PR fix: avoid npm install lifecycle conflict #1)
  • npx mocha test/default_logger_test.js test/logging/request-obfuscator_test.js --require should
  • npx eslint src/index.js test/default_logger_test.js
  • git diff --check

Note: the full pre-commit hook runs eslint src test and fails in this Windows checkout because many untouched repository files are checked out with CRLF while the legacy lint rule requires LF. The targeted changed files pass ESLint after autofix.

Refs BitgesellOfficial/bitgesell#39. Payout details can be provided privately if maintainers consider this eligible under the bounty program.

Copilot AI review requested due to automatic review settings May 25, 2026 08:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the default debug logger namespace used by the client and adds a test to ensure this default remains correct.

Changes:

  • Change default debugnyan namespace from bitcoin-core to bitgesell-core.
  • Add a regression test that asserts the default logger namespace.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/default_logger_test.js Adds a test that monkey-patches debugnyan to capture and assert the namespace.
src/index.js Updates the default logger namespace string.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +19 to +20
delete require.cache[clientPath];
require.cache[debugnyanPath].exports = name => {
@MyTH-zyxeon

Copy link
Copy Markdown

Maintainer triage for the Bitgesell bounty queue (#39): this looks like a narrow, relevant Bitgesell-branding fix rather than a broad dependency or generated-file change.

What I verified from the PR diff:

  • The runtime change is only the default logger namespace in src/index.js, from bitcoin-core to bitgesell-core.
  • The added regression test stubs debugnyan and asserts that constructing the default client captures bitgesell-core.
  • The existing Copilot concern about require.cache[debugnyanPath] being undefined appears addressed by the test as written: const originalDebugnyan = require(debugnyanPath); runs before require.cache[debugnyanPath].exports = ..., so the cache entry should exist before mutation.

Recommended maintainer acceptance checks before merge:

  • Run the focused test command from the PR body.
  • Run ESLint on src/index.js and test/default_logger_test.js.
  • Confirm whether the repository wants the debug namespace aligned with the documented/package branding everywhere, or only for this default constructor path.

I did not run the package locally in this pass, and I am not making a payout claim from this review note.

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.

3 participants