feat(cli): improve publish command output

[noahlitvin]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: b9c0441

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

[saturn-dbeal]

Tests pass (6/6). The output formatting improvements are nice — cleaner structure with separators and better readability. However, there are a few issues that should be addressed:

1. Balance check moved inside !quiet block — The insufficient balance check is now gated by if (!quiet), meaning in quiet mode the user won't get the early error and will instead get a cryptic transaction failure. The balance check should always run.

2. Breaking change to function signature — The tags parameter was moved before onChainRegistry in the function signature. This is a breaking change for any callers using positional arguments. Not necessarily wrong, but worth documenting if intentional.

3. Changed from toStorage.registry.publishMany to onChainRegistry.publishMany — This bypasses the storage abstraction layer. If toStorage.registry was ever a different registry implementation than onChainRegistry, this would change behavior. Looks like they're always the same in practice, but the change is worth calling out.

[saturn-dbeal]

The balance check is now inside if (!quiet), meaning in quiet mode the insufficient balance check is skipped. The transaction will still fail, but the user gets a less helpful error. Consider moving the balance check outside the !quiet block:

if (onChainRegistry instanceof OnChainRegistry) {
  const totalFees = await onChainRegistry.calculatePublishingFee(publishCalls.length);
  
  if (totalFees > 0n) {
    const balance = await onChainRegistry.provider!.getBalance({ 
      address: onChainRegistry.signer!.address 
    });
    
    if (totalFees >= balance) {
      throw new Error('You do not appear to have enough ETH in your wallet to publish');
    }
  }
  
  if (!quiet) {
    log(bold('\nTransaction Details:'));
    log(gray(`  Publishing Fee: ${viem.formatEther(totalFees)} ETH`));
  }
}

[saturn-dbeal]

Good change — throw new Error('User aborted') is much better than process.exit(1) for testability and proper error handling upstream.

[saturn-dbeal]

Good change — returning the txHashes makes this function more useful for programmatic consumers.

[saturn-dbeal]

Tests pass (6/6). The output formatting improvements are nice — cleaner structure with separators and better readability. However, there are a few issues that should be addressed:

1. Balance check moved inside !quiet block — The insufficient balance check is now gated by if (!quiet), meaning in quiet mode the user won't get the early error and will instead get a cryptic transaction failure. The balance check should always run.

2. Breaking change to function signature — The tags parameter was moved before onChainRegistry in the function signature. This is a breaking change for any callers using positional arguments. Not necessarily wrong, but worth documenting if intentional.

3. Changed from toStorage.registry.publishMany to onChainRegistry.publishMany — This bypasses the storage abstraction layer. If toStorage.registry was ever a different registry implementation than onChainRegistry, this would change behavior. Looks like they're always the same in practice, but the change is worth calling out.

[saturn-dbeal]

The balance check is now inside if (!quiet), meaning in quiet mode the insufficient balance check is skipped. The transaction will still fail, but the user gets a less helpful error. Consider moving the balance check outside the !quiet block:

if (onChainRegistry instanceof OnChainRegistry) {
  const totalFees = await onChainRegistry.calculatePublishingFee(publishCalls.length);
  
  if (totalFees > 0n) {
    const balance = await onChainRegistry.provider!.getBalance({ 
      address: onChainRegistry.signer!.address 
    });
    
    if (totalFees >= balance) {
      throw new Error('You do not appear to have enough ETH in your wallet to publish');
    }
  }
  
  if (!quiet) {
    log(bold('\nTransaction Details:'));
    log(gray(`  Publishing Fee: ${viem.formatEther(totalFees)} ETH`));
  }
}

[saturn-dbeal]

Good change — throw new Error('User aborted') is much better than process.exit(1) for testability and proper error handling upstream.

[saturn-dbeal]

Good change — returning the txHashes makes this function more useful for programmatic consumers.

[saturn-dbeal]

Tests pass (6/6). The output formatting improvements are nice — cleaner structure with separators and better readability. However, there are a few issues that should be addressed:

1. Balance check moved inside !quiet block — The insufficient balance check is now gated by if (!quiet), meaning in quiet mode the user won't get the early error and will instead get a cryptic transaction failure. The balance check should always run.

2. Breaking change to function signature — The tags parameter was moved before onChainRegistry in the function signature. This is a breaking change for any callers using positional arguments. Not necessarily wrong, but worth documenting if intentional.

3. Changed from toStorage.registry.publishMany to onChainRegistry.publishMany — This bypasses the storage abstraction layer. If toStorage.registry was ever a different registry implementation than onChainRegistry, this would change behavior. Looks like they're always the same in practice, but the change is worth calling out.

[saturn-dbeal]

Tests pass (6/6). The output formatting improvements are nice — cleaner structure with separators and better readability. However, there are a few issues that should be addressed:

1. Balance check moved inside !quiet block — The insufficient balance check is now gated by if (!quiet), meaning in quiet mode the user won't get the early error and will instead get a cryptic transaction failure. The balance check should always run.

2. Breaking change to function signature — The tags parameter was moved before onChainRegistry in the function signature. This is a breaking change for any callers using positional arguments. Not necessarily wrong, but worth documenting if intentional.

3. Changed from toStorage.registry.publishMany to onChainRegistry.publishMany — This bypasses the storage abstraction layer. If toStorage.registry was ever a different registry implementation than onChainRegistry, this would change behavior. Looks like they're always the same in practice, but the change is worth calling out.