Skip to content

Conversation

@steviez
Copy link
Collaborator

@steviez steviez commented Oct 24, 2025

Remove the dependency and the couple info!() statements from unit tests since they would no longer print without a logger enabled

The info! statements seemed non-essential so I decided to remove them; we could leave them if we manually call env_logger to initialize a logger. I don't feel too strongly one way or another there.

This is the last use of solana-logger in this crate; I will remove solana-logger completely after this PR lands

Remove the dependency and the couple info!() statements from unit tests
since they would no longer print without a logger enabled
@github-actions
Copy link
Contributor

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

@steviez steviez requested a review from joncinque October 24, 2025 17:09
Copy link
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Just one comment

@github-actions
Copy link
Contributor

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

Copy link
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks great!

@steviez
Copy link
Collaborator Author

steviez commented Oct 24, 2025

Thanks for catching that issue. Since this was only a dev dependency, I don't believe we need to push a new tag. Do you agree with that or am I overlooking anything ?

@steviez steviez merged commit 40218b9 into anza-xyz:master Oct 24, 2025
27 checks passed
@steviez steviez deleted the fee_calc_rm_logger branch October 24, 2025 21:36
@joncinque
Copy link
Collaborator

Thanks for catching that issue. Since this was only a dev dependency, I don't believe we need to push a new tag. Do you agree with that or am I overlooking anything ?

Yep that's correct, no need to do anything more

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