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

Re-enable spl downstream build #32429

Merged

Conversation

KirillLykov
Copy link
Contributor

Problem

Re-enable spl downstream build which was disabled after borsh upgrade.
See #32378

Summary of Changes

@yihau
Copy link
Member

yihau commented Jul 9, 2023

Thank you <3

btw, I'm thinking do we want to make this one happen in this same PR. 072884d

I modified it accidentally but the fix hasn't merged cuz some issues. 😢

Copy link
Member

@yihau yihau left a comment

Choose a reason for hiding this comment

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

(btw, this PR is good enough to me! I can fix the error in my original PR if it doesn't happen in this one!)

@KirillLykov
Copy link
Contributor Author

Thank you <3

btw, I'm thinking do we want to make this one happen in this same PR. 072884d

I modified it accidentally but the fix hasn't merged cuz some issues. 😢

My understanding is that you propose to run test instead of build. I'm not sure about doing cargo-test-sbf because there is timeout 30min currently and I don't know how long test will take.
I think we should not mix re-enabling build and enabling tests because each change might go wrong for some reason and it will be harder to track.

@yihau
Copy link
Member

yihau commented Jul 9, 2023

yeah. no problem. I can fix it in another PR.
tbh, we did test before. one of my reorg PR broke it.

$cargo_test_bpf --manifest-path "$program"/Cargo.toml

(^^^ from the v1.14 branch)

Copy link
Contributor

@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! It's too bad it doesn't run the job in this PR though

@KirillLykov KirillLykov merged commit 48ac48e into solana-labs:master Jul 10, 2023
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.

3 participants