Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract transaction-context crate #3132

Merged
merged 12 commits into from
Nov 19, 2024

Conversation

kevinheavey
Copy link

Problem

solana_sdk::transaction_context imposes a solana_sdk dep on solana_transaction_status_client_types

Summary of Changes

  • Move to its own crate and re-export with deprecation notice
  • transaction_context.rs was making use of the full feature of solana-sdk for code using debug_assertions. Add a debug_assertions feature to the new crate to replace the full feature.
  • Make serde and bincode optional in the new crate
  • Add doc_auto_cfg like in add doc_auto_cfg to relevant sdk crates #3121

@kevinheavey kevinheavey force-pushed the extract-transaction-context branch 3 times, most recently from 6b718a4 to 04c3850 Compare October 15, 2024 16:15
@kevinheavey kevinheavey force-pushed the extract-transaction-context branch 3 times, most recently from a97f5ff to a2ca822 Compare October 22, 2024 18:42
@kevinheavey kevinheavey force-pushed the extract-transaction-context branch 3 times, most recently from 989ee76 to f30e79e Compare October 29, 2024 22:19
@kevinheavey kevinheavey force-pushed the extract-transaction-context branch from f30e79e to cc7162c Compare November 5, 2024 11:07
@kevinheavey kevinheavey force-pushed the extract-transaction-context branch from cc7162c to 518a4f8 Compare November 12, 2024 11:03
Copy link

@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 overall! Just a question on the name of the feature, I think you took the right approach.

I looked for another potential crate to host this type, but it's really used all over the place, so a separate crate makes sense.

sdk/transaction-context/Cargo.toml Outdated Show resolved Hide resolved
Copy link

@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.

Sorry, one last bit

Copy link

@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 good to me! @yihau can you accept solana-transaction-context?

@yihau yihau merged commit 45bdc18 into anza-xyz:master Nov 19, 2024
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants