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

Use --locked for cargo commands && change Cargo.toml to use crates.io #344

Merged
merged 5 commits into from
Jan 20, 2025

Conversation

CyonAlexRDX
Copy link
Contributor

@CyonAlexRDX CyonAlexRDX commented Jan 16, 2025

Also rename github worksflows file names which were confusing.

For more context see Slack.

I think I've updated all cargo commands, but please review that, especially @micbakos-rdx for the gradle files.

I've also switched to using default-features = false everywhere

Cargo.toml Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was unable to use version for Scrypto crates since radix-engine-toolkit-common was not on crates.io

@@ -53,7 +53,7 @@ impl Mnemonic {
let language = internal.language();

let words = internal
.word_iter()
.words()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

word_iter method was deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Witness:

  • I've added --locked to all three cargo build commands (iOS device, iOS sim, macOS Apple Silicon)
  • I've added --locked to cargo run sargon-bindgen for iOS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Witness:

  • I've added --locked to the cargo build command for Kotlin
  • I've added --locked to cargo run sargon-bindgen for Kotlin (in file jvm/sargon-android/build.gradle.kts below)

Copy link
Contributor Author

@CyonAlexRDX CyonAlexRDX Jan 16, 2025

Choose a reason for hiding this comment

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

Witness - I've added --locked to:

  • cargo check
  • cargo fmt - did not work, so not added.
  • cargo clippy
  • cargo run nextest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Witness - iOS workflow does not perform any cargo command, it only calls scripts/ios/build-sargon.sh which has been modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Witness - Android workflow does not perform any cargo command, it only calls gradle which has been modified. (both CargoDesktopPlugkin.kt). Hmm @micbakos-rdx there is no corresponding "CargoAndroidPlugin.kt" right? We use CargoDesktopPlugkin.kt for Android too right?

@@ -137,7 +137,7 @@ jobs:
components: rustfmt

- name: Check formatting
run: cargo fmt --check
run: cargo fmt --check --all
Copy link
Contributor Author

Choose a reason for hiding this comment

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

--locked did not work for cargo fmt, so skipped that, but added --all for symmetry (I think I've previously asserted that it was not needed?)

Copy link

codecov bot commented Jan 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.84%. Comparing base (28ac8b8) to head (8e63767).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #344   +/-   ##
=======================================
  Coverage   92.84%   92.84%           
=======================================
  Files        1160     1160           
  Lines       25797    25797           
  Branches       85       85           
=======================================
  Hits        23952    23952           
  Misses       1834     1834           
  Partials       11       11           
Flag Coverage Δ
kotlin 97.88% <ø> (ø)
rust 92.37% <ø> (ø)
swift 93.76% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CyonAlexRDX CyonAlexRDX force-pushed the ac/cargo_version_ci_locked branch from fc0d5c1 to d4d19c2 Compare January 16, 2025 12:11
@CyonAlexRDX CyonAlexRDX merged commit eca2370 into main Jan 20, 2025
13 checks passed
@CyonAlexRDX CyonAlexRDX deleted the ac/cargo_version_ci_locked branch January 20, 2025 09:20
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