revanced-website icon indicating copy to clipboard operation
revanced-website copied to clipboard

fix: Sort versions consistently

Open Ushie opened this issue 1 year ago • 14 comments

I couldn't figure out why these lines of code were here, or if removing them would effect anything, but it seems to work perfectly fine Closes #238

Ushie avatar Apr 28 '24 23:04 Ushie

Real problem is that semver doesn't handle versions with leading 0s, and as such, labels those as invalid.

I couldn't figure out why these lines of code were here

To sort all the invalid versions to the start I think

KTibow avatar May 02 '24 22:05 KTibow

What do you mean? The version is not according to semver specs.

oSumAtrIX avatar May 02 '24 22:05 oSumAtrIX

Yes, yet we rely on the semver package for sorting the items.

KTibow avatar May 02 '24 22:05 KTibow

That should be removed then

oSumAtrIX avatar May 02 '24 22:05 oSumAtrIX

removed

Are you saying sorting the supported versions should be entirely disabled? Or to replace it instead of entirely removing it?

KTibow avatar May 02 '24 22:05 KTibow

I am saying the version should not be parsed as semver

oSumAtrIX avatar May 02 '24 22:05 oSumAtrIX

Well, please explicitly state if you mean it should be parsed another way (most likely) or not parsed (literal interpretation of what you're saying)

KTibow avatar May 02 '24 22:05 KTibow

There's no requirement for sorting via the version name

oSumAtrIX avatar May 02 '24 22:05 oSumAtrIX

How else do you propose we sort it?

Ushie avatar May 03 '24 00:05 Ushie

As stated in my previous comment, there is no requirement to sort by version name at all.

oSumAtrIX avatar May 03 '24 01:05 oSumAtrIX

Are you saying we should use the sorting provided by the backend instead? If so, that sounds like you should make a separate issue for that

KTibow avatar May 03 '24 01:05 KTibow

The backend does not specify any sorting and is considered to be random. No issue is necessary

oSumAtrIX avatar May 03 '24 01:05 oSumAtrIX

What I'm saying is if you want to remove the current sorting, you should discuss that in a place like an issue first.

KTibow avatar May 03 '24 01:05 KTibow

Since it's discussed here with a clear solution, that won't be necessary anymore and this PR can be repurposed to solve both issues at once

oSumAtrIX avatar May 03 '24 01:05 oSumAtrIX