Sort kegs based on version scheme #16973
Conversation
This comment was marked as spam.
This comment was marked as spam.
This comment has been minimized.
This comment has been minimized.
|
Nit: It might make more sense to have |
|
Good catch, thanks for this @Bo98! |
| head - [Keg.new(head_prefix)] + stable.sort_by(&:version).slice(0...-1) | ||
| head - [Keg.new(head_prefix)] + Keg.sort(stable).drop(1) |
There was a problem hiding this comment.
I think this logic is different from before since we're now doing a reverse sort by version. Before we were dropping the newest version and now we're dropping the oldest one.
There was a problem hiding this comment.
slice(0...-1) drops the last entry but drop(1) drops the first entry
| sig { params(kegs: T::Array[Keg]).returns(T::Array[Keg]) } | ||
| def self.sort(kegs) | ||
| kegs.sort_by { |keg| [keg.version_scheme, keg.version] }.reverse! | ||
| end |
There was a problem hiding this comment.
This is not equivalent to the previous usages of keg_array.sort_by(&:version) since it now does a reverse sort. Are you sure that order doesn't matter in any of the places where it was replaced?
There was a problem hiding this comment.
JSON API maybe matters, but that's about it. Most of the rest don't use the full list and those were all updated, and the others were largely in non-parsable output (e.g. plain brew info/brew outdated).
|
I opened a follow-up PR here: #16986 |
version_schemeis exclusively used in situations where new version < old version, so all keg sorting techniques need to be aware of it.Because this requires reading the tab, this has a minor performance penalty. Hopefully it is not noticeable.
This is a best effort attempt to fix all existing call sites - I could have missed some.