Skip to content

Conversation

@arobsn
Copy link
Member

@arobsn arobsn commented May 22, 2025

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented May 22, 2025

⚠️ No Changeset found

Latest commit: 2248189

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codecov
Copy link

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (2f63718) to head (2248189).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #187   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          123       123           
  Lines        14038     14036    -2     
  Branches      1729      1727    -2     
=========================================
- Hits         14038     14036    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new
Copy link

pkg-pr-new bot commented May 22, 2025

@fleet-sdk/blockchain-providers

npm i https://pkg.pr.new/@fleet-sdk/blockchain-providers@187

@fleet-sdk/common

npm i https://pkg.pr.new/@fleet-sdk/common@187

@fleet-sdk/compiler

npm i https://pkg.pr.new/@fleet-sdk/compiler@187

@fleet-sdk/core

npm i https://pkg.pr.new/@fleet-sdk/core@187

@fleet-sdk/crypto

npm i https://pkg.pr.new/@fleet-sdk/crypto@187

@fleet-sdk/mock-chain

npm i https://pkg.pr.new/@fleet-sdk/mock-chain@187

@fleet-sdk/serializer

npm i https://pkg.pr.new/@fleet-sdk/serializer@187

@fleet-sdk/wallet

npm i https://pkg.pr.new/@fleet-sdk/wallet@187

@fleet-sdk/ageusd-plugin

npm i https://pkg.pr.new/@fleet-sdk/ageusd-plugin@187

@fleet-sdk/babel-fees-plugin

npm i https://pkg.pr.new/@fleet-sdk/babel-fees-plugin@187

commit: 2248189

@arobsn arobsn requested a review from Copilot May 22, 2025 17:52
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for merging additional registers in the OutputBuilder by updating the management of register assignments and validation.

  • Merges new registers directly into the builder’s internal register field.
  • Moves the dense packing validation from the setter to the build method.

Reviewed Changes

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

File Description
packages/core/src/builder/outputBuilder.ts Updates register assignment and validation approach in setAdditionalRegisters and build methods.
packages/core/src/builder/outputBuilder.spec.ts Adds tests to verify merging behavior and proper error throwing when registers are missing.
Comments suppressed due to low confidence (1)

packages/core/src/builder/outputBuilder.spec.ts:350

  • [nitpick] Consider breaking the chained calls (setAdditionalRegisters() followed by build()) into separate statements to improve the readability of the test cases.
outputBuilder.setAdditionalRegisters({

@arobsn arobsn merged commit 35f0250 into master May 22, 2025
16 checks passed
@arobsn arobsn deleted the improve-builder-registers branch May 22, 2025 17:55
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.

2 participants