feat: Support a fixed launcher version #358
Conversation
| } else { | ||
| format!("v{version}") | ||
| version.to_owned() | ||
| }; |
There was a problem hiding this comment.
} else if version.starts_with('v') {
version.to_owned()
} else {
format!("v{version}")
};
The v-prefix normalization currently lives only in the From<Mode> for Configuration impl in mod.rs. If cache.rs is ever called from a different path (or someone constructs a ManagedLauncherConfig directly with a bare version like "12.0.0"), it’ll look up the wrong cache directory and try to download a nonexistent release. Can we add defensive normalization here too?
There was a problem hiding this comment.
My goal here was to avoid needing defensive normalization everywhere that touches the version including adding it to new places that touch it, and instead pipeline it through exactly one spot.
When we deal with the config, it's always in canonical form, but when it's altered by the user, it's always in manifest form; performing things like this is a lot of the point of having the two forms, you similarly don't need more than one place in the code that fills in the default value for an optional field. I don't see the harm in adding an assertion, but I think accommodating it as a code path is defeating the point of the normalization.
| } else { | ||
| format!("v{version_req}") | ||
| version_req.to_owned() | ||
| }; |
There was a problem hiding this comment.
} else if version.starts_with('v') {
version.to_owned()
} else {
format!("v{version}")
};
Same here.
3 participants
No description provided.