[CPP-347] Improve handling of firmware versions #174
Conversation
Introduces a new 'SwiftVersion' class that can be used for comparing firmware and console versions. This was heavily inspired/ported from the old console and removes the need for the SemVer package. As part of this, firmware upgrades from before v2.0.0 to after v2.0.0 are not allowed, as in the old console.
console_backend/src/swift_version.rs
Outdated
|
|
||
| impl PartialEq for SwiftVersion { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| self.marketing == other.marketing && self.major == other.major && self.minor == other.minor |
There was a problem hiding this comment.
This seems to treat all dev versions as equivalent.
With regards to namespaces, would this treat Starling 3.0 the same as Piksi Multi 3.0?
There was a problem hiding this comment.
Yeah it does, this is a simplification that makes sense with the way we use the class (where we don't ever compare dev versions) but probably doesn't make sense otherwise.
If we don't do this then with the current implementation SwiftVersion::parse("1.1.1-dev") != SwiftVersion::parse("1.1.1") but SwiftVersion::parse("1.1.1-dev").cmp(SwiftVersion::parse("1.1.1")) == Ordering.Equal.
I think the "right" way of handling this might be to only implement PartialOrd and then only allow versions to be compared if the namespace and dev parts are the same. I guess it's probably worth doing in case this class gets used for more than it's currently used for, so that there aren't surprising results. I was just trying to avoid the verbosity of having to check that the comparisons are Some everywhere they're performed.
What do you think @silverjam ?
There was a problem hiding this comment.
I think the "right" way of handling this might be to only implement
PartialOrdand then only allow versions to be compared if the namespace and dev parts are the same. I guess it's probably worth doing in case this class gets used for more than it's currently used for, so that there aren't surprising results. I was just trying to avoid the verbosity of having to check that the comparisons areSomeeverywhere they're performed.
Yes, this is exactly what I was thinking too.
2 participants
Introduces a new 'SwiftVersion' class that can be used for comparing firmware and console versions. This was heavily inspired/ported from the old console and removes the need for the SemVer package.
As part of this, firmware upgrades from before v2.0.0 to after v2.0.0 are not allowed, as in the old console.