Check the rustup version #248
Conversation
561940f to
059e74b
Compare
There was a problem hiding this comment.
The code looks fine to me but I think the constraint ">= 1.23.0" should be part of the dynamic configuration (e.g. inside the language.rust section of https://developer.fastly.com/api/internal/cli-config).
This means updating https://github.com/fastly/cli/blob/master/pkg/config/data.go#L192-L204 to reflect a new rustup_constraint key set to >= 1.23.0, while also updating the internal dynamic configuration repo to publish an updated config with this value added.
NOTE: the reason I suggest including the constraint syntax
>=and not just the expected version1.23.0is because it provides more control to the team responsible for handling the rust dependencies (for example, the team had previously requested forfastly_sys_constraintto be set to ">= 0.3.3 < 0.5.0" rather than just a specific version). I appreciate that was to solve a different kind of coupling issue but I think I still the prefer the flexibility of this being inside the dynamic config if a change was necessary in future.
cc @acfoltzer and @cratelyn
I was not aware of this but I agree. I will update the PR. Thank you. |
d5fe2c1 to
381b5ac
Compare
Turns out, the toolchain section is only available in rustup 1.23.0 and higher. Fixes #137.
Co-authored-by: Mark McDonnell <Integralist@users.noreply.github.com>
381b5ac to
a7ee451
Compare
Suggested by @Integralist.
a7ee451 to
d96e0ba
Compare
|
Looks good from the rustup semantics POV, thanks for the cc @Integralist! |
Turns out, the toolchain section is only available in rustup 1.23.0 and higher.
Fixes #137.