Skip to content

Conversation

@zees-dev
Copy link
Contributor

@zees-dev zees-dev commented Jul 31, 2024

Context

Ideally we should migrate to stable rust for toolchain stability; this PR achieves that.

  • There are discrepancies in the CI image used to produce the release and the rust version specified in the the rust-toolchain.toml
  • The runtime wasm output/artifact uploaded for releases is currently the debug WASM (not release/optimized)
  • Stable rust does not support the same rustfmt.toml as nightly; hence there's a bunch of formatting changes in this PR
    • Note all the changes to the .rs files are formatting changes due to the toolchain update
  • The xrpl-sdk-rust main branch supports stable, hence the dep is updated it point to that branch; diff here

@zees-dev zees-dev requested a review from JasonTulp July 31, 2024 12:29
Copy link
Contributor

@JasonTulp JasonTulp left a comment

Choose a reason for hiding this comment

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

lgtm!

- name: Check out
uses: actions/checkout@v3
- name: Install toolchain
run: rustup show
Copy link
Contributor

Choose a reason for hiding this comment

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

Why get rid of rustup show line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesnt actually install the toolchain; just shows it. Using GH action instead to deterministically install the correct toolchain.

Copy link
Contributor

@surangap surangap left a comment

Choose a reason for hiding this comment

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

lgtm!

@zees-dev zees-dev merged commit 5e3703f into main Aug 1, 2024
@zees-dev zees-dev deleted the upd/rust-stable-1.80 branch August 1, 2024 23:53
@surangap surangap mentioned this pull request Aug 6, 2024
4 tasks
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.

4 participants