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 program-error crate #2413

Merged
merged 10 commits into from
Sep 16, 2024
Merged

Conversation

kevinheavey
Copy link

Problem

solana_program::program_error needs to be made available outside solana-program

This branches off #2405 so that needs to be merged first.

Summary of Changes

  • move program_error.rs to its own crate and re-export its contents for backwards compatibility
  • make serde optional in the new crate
  • remove thiserror from the new crate

@kevinheavey kevinheavey changed the title Extract program error Extract program-error crate Aug 2, 2024
Copy link

mergify bot commented Aug 9, 2024

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

@kevinheavey kevinheavey force-pushed the extract-program-error branch 2 times, most recently from af4f8ac to f693775 Compare September 7, 2024 13:36
@kevinheavey kevinheavey marked this pull request as ready for review September 13, 2024 17:08
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! Just the one missing space, but it looks like rustfmt doesn't care. @yihau can you accept solana-program-error?

@@ -5,22 +5,20 @@
//! [`bpf_loader`]: crate::bpf_loader

extern crate alloc;
pub use solana_program_error::ProgramResult;

Choose a reason for hiding this comment

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

Gosh, this makes so much more sense

Choose a reason for hiding this comment

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

I agree with your approach of not making this no-std -- if we ever want to do it, we can introduce a breaking change later.

sdk/program-error/src/lib.rs Outdated Show resolved Hide resolved
Co-authored-by: Jon C <me@jonc.dev>
@yihau
Copy link
Member

yihau commented Sep 14, 2024

crate-check passed!

@kevinheavey kevinheavey added the automerge automerge Merge this Pull Request automatically once CI passes label Sep 14, 2024
@mergify mergify bot merged commit 2a962c9 into anza-xyz:master Sep 16, 2024
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge automerge Merge this Pull Request automatically once CI passes need:merge-assist
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants