Skip to content

Fix internalLogger.decToHex function#167

Merged
kikoveiga merged 1 commit into
developfrom
kikoveiga/fix-dec-to-hex-function
Jun 2, 2026
Merged

Fix internalLogger.decToHex function#167
kikoveiga merged 1 commit into
developfrom
kikoveiga/fix-dec-to-hex-function

Conversation

@kikoveiga

@kikoveiga kikoveiga commented May 29, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Simplifies and fixes the decimal to hexadecimal helper function, which had a bug because it was using / instead of \ division.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@kikoveiga
kikoveiga marked this pull request as ready for review May 29, 2026 14:19
@kikoveiga
kikoveiga requested a review from a team as a code owner May 29, 2026 14:19
@kikoveiga kikoveiga self-assigned this May 29, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@kikoveiga
kikoveiga force-pushed the kikoveiga/fix-dec-to-hex-function branch from b1ec84c to f282b28 Compare May 29, 2026 14:22
@ambushwork
ambushwork requested a review from Copilot June 1, 2026 06:23

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

Fixes the decToHex helper used by internal logging/error formatting by replacing the manual conversion loop (which relied on / division) with the built-in integer-to-string base conversion, and adds a dedicated unit test suite to lock in expected output.

Changes:

  • Simplified decToHex(number) to UCase(StrI(number, 16).Trim()) in both source and dist outputs.
  • Added unit tests covering common values and the max signed 32-bit integer.
  • Registered the new InternalLogger test suite in the test runner.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
test/source/tests/Test__internalLogger.bs Adds unit coverage for decimal-to-hex conversion outputs.
test/components/TestRunner.bs Wires the new InternalLogger test suite into the test run.
library/source/internalLogger.bs Replaces manual dec→hex conversion with StrI(..., 16) + normalization.
dist/source/internalLogger.brs Mirrors the same decToHex implementation change in the dist artifact.

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

@kikoveiga
kikoveiga merged commit 109a0d3 into develop Jun 2, 2026
9 checks passed
@kikoveiga
kikoveiga deleted the kikoveiga/fix-dec-to-hex-function branch June 2, 2026 09:58
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