-
Notifications
You must be signed in to change notification settings - Fork 23
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
[MTG-1007] Updated solana version to 2 #85
Conversation
e650711
to
5e31e99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks legit
c7be7fe
to
b273824
Compare
Changed imports respectively; Updated Dockerfile and docs; Changed methods to match the required interface;
b273824
to
cd2a712
Compare
494bbee
to
ba1dee8
Compare
fi | ||
|
||
if [[ -n $RUST_NIGHTLY_VERSION ]]; then | ||
nightly_version="$RUST_NIGHTLY_VERSION" | ||
else | ||
nightly_version=2022-04-01 | ||
nightly_version=latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why latest for both cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not latest rather?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is latest
a valid tag for rustup? Shouldn't these just be stable
and nightly
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That tag is not for rustup
. Both nightly
and stable
are indeed valid toolchain values for a "default" rustup, however, the repo uses images from solanalabs. Those images do not contain toolchains for both versions simultaneously. For instance, the image with nightly
was updated one year ago. Therefore, the latest
tag is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok sg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not following why we use latest as both stable and nightly, otherwise, looks great
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with one comment to the rust version used in CI testing
The main reason for updating is to provide a possibility of running the Solana node with version 2.