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

build: add rust-toolchain.toml file #425

Closed
wants to merge 1 commit into from

Conversation

tdelabro
Copy link
Contributor

Let me know what you think but, especially for open source, I prefer to pin the version of the language to make sure that all contributors are working with the same one and that no bugs or changes in behaviors they may encounter are due to running a different version of the language

Run cargo install at the project root to download and install the language version and the components we listed.

@thesimplekid
Copy link
Collaborator

That's why I use the nix shells, so I won't use this but I'm fine with adding it in addition. Can the channel be stable instead of a specific version?

@tdelabro
Copy link
Contributor Author

Can the channel be stable instead of a specific version?

It is not a good idea, it would mean that the project toolchain is automatically updated every time a new version is released, without any human check to see if nothing has been broken during this release. One day it will just refuse to compile because some breaking change has been introduced that we don't handle

@tdelabro
Copy link
Contributor Author

That's why I use the nix shells,

I can see the version being defined there: https://github.com/cashubtc/cdk/blob/main/flake.nix
We have to use the same in both, otherwise it loses its interest.

It looks like you are using stable latest in it:

stable_toolchain = pkgs.rust-bin.stable.latest.default.override {
          targets = [ "wasm32-unknown-unknown" ]; # wasm
        };

but with target = wasm32 ??? Is it the only thing you compile too? Not native, only wasm?

@thesimplekid
Copy link
Collaborator

but with target = wasm32 ??? Is it the only thing you compile too? Not native, only wasm?

It add the was wasm target, native is already included.

The CI also uses the nix flake, checks both wasm and native on stable and MSRV

@tdelabro
Copy link
Contributor Author

So the only thing I would recommend would be to fix the version of the language you are using in the CLI.
And in the future make sure it's the same for both the toolchain file and the nix one

@thesimplekid
Copy link
Collaborator

closed for #434

I set a specific rust-version in the flake and toolchain.toml (current stable 1.82.0)

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.

2 participants