Add error for volta install 10
#1020
Conversation
charlespierce
left a comment
charlespierce
left a comment
There was a problem hiding this comment.
Thanks for this! A couple of small tweaks but looks like a great usability improvement!
crates/volta-core/src/tool/serial.rs
Outdated
| name: name.as_ref().to_string(), | ||
| version: maybe_version.as_ref().to_string(), | ||
| match (args.next(), args.next(), args.next()) { | ||
| (Some(maybe_version), None, None) => { |
There was a problem hiding this comment.
If you'll take a random question while I'm here (I'm new to rust) - would it be more idiomatic to have
(Some(maybe_version), None, None) if is_version_like(maybe_version.as_ref()) => {rather than
(Some(maybe_version), None, None) => {
if is_version_like(maybe_version.as_ref()) {?
(In the case that nothing else is going to match anyway so that they have the same semantics.)
Also, if it is, would it be more idiomatic to have the match itself provide the return value, i.e. get rid of the returns and make the _ branch be _ => Ok(())?
Happy to push up a commit with that change if it would be an improvement.
There was a problem hiding this comment.
That's a great question! In my experience, it usually comes down to readability: I find the <pattern> if <predicate> => can very easily get verbose enough that it's more difficult to read than <pattern> => if <predicate>. For your second question, yes if you use the predicate matching it would be more idiomatic to have the match provide the value directly.
In this case, I would probably lean that doing predicate matching is slightly cleaner, but it's not anything that would block a PR. So if you'd like to (and have the time to) add another commit, that would be a bonus! But if not no worries and I'm happy to merge as is 👍
There was a problem hiding this comment.
Thanks for the response! Pushed up a commit doing that.
|
Awesome, thanks! |
|
@charlespierce Any chance of cutting a release with this change? |
|
@bakkot Done! Released as part of 1.0.5! |
2 participants
Fixes #1016.
I didn't document this anywhere, since it seems like something you don't need to know about until you've already tripped over it.