Skip to content

Extract loader-v2-interface crate#4487

Merged
joncinque merged 4 commits into
anza-xyz:masterfrom
kevinheavey:extract-loader-instruction
Jan 22, 2025
Merged

Extract loader-v2-interface crate#4487
joncinque merged 4 commits into
anza-xyz:masterfrom
kevinheavey:extract-loader-instruction

Conversation

@kevinheavey
Copy link
Copy Markdown

Problem

solana_program::loader_instruction imposes a solana_program dep on solana_transaction_status.

Summary of Changes

  • Move to a new crate and re-export for backwards compatibility. I called this one solana-loader-instruction instead of solana-loader-interface because the instruction module is the only thing in the crate
  • Update solana-transaction-status to use the new crate

@kevinheavey kevinheavey force-pushed the extract-loader-instruction branch from 70581d4 to f35686a Compare January 16, 2025 00:52
Copy link
Copy Markdown

@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! Just the naming bit

Comment thread sdk/loader-instruction/Cargo.toml Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: can we call this solana-loader-v2-interface? the v2 bit so that people are less tempted to use it, especially when they see v3 and v4, and interface for consistency with the other loader crates, even though there's no state.

I could even be convinced to go with v1 since they share an interface.

Comment thread sdk/loader-instruction/Cargo.toml Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is this feature needed?

@kevinheavey kevinheavey changed the title Extract loader-instruction crate Extract loader-v2-interface crate Jan 22, 2025
joncinque
joncinque previously approved these changes Jan 22, 2025
Copy link
Copy Markdown

@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, thanks!

@joncinque
Copy link
Copy Markdown

@yihau can you accept ownership of solana-loader-v2-interface?

@kevinheavey kevinheavey force-pushed the extract-loader-instruction branch from 5ffebc6 to c4bd2c2 Compare January 22, 2025 11:56
@joncinque joncinque merged commit 30735d7 into anza-xyz:master Jan 22, 2025
@Lichtso
Copy link
Copy Markdown

Lichtso commented Jan 28, 2025

loader-v2 program management has been deactivated for a while (a year or longer). Why not remove it entirely?

@kevinheavey
Copy link
Copy Markdown
Author

would be a breaking change in solana-program and solana-sdk

@Lichtso
Copy link
Copy Markdown

Lichtso commented Jan 28, 2025

Yes, but what about the deprecated attribute. It says you should use the moved crate. But it should say that this functionality is gone and will be completely removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants