ReVanced / revanced-website

🌐 Website for ReVanced
https://revanced.app
GNU General Public License v3.0
279 stars 43 forks source link

fix: Remove unnecessary app version sorting #239

Open Ushie opened 2 months ago

Ushie commented 2 months ago

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

KTibow commented 1 month ago

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

oSumAtrIX commented 1 month ago

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

KTibow commented 1 month ago

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

oSumAtrIX commented 1 month ago

That should be removed then

KTibow commented 1 month ago

removed

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

oSumAtrIX commented 1 month ago

I am saying the version should not be parsed as semver

KTibow commented 1 month ago

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)

oSumAtrIX commented 1 month ago

There's no requirement for sorting via the version name

Ushie commented 1 month ago

How else do you propose we sort it?

oSumAtrIX commented 1 month ago

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

KTibow commented 1 month ago

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

oSumAtrIX commented 1 month ago

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

KTibow commented 1 month ago

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

oSumAtrIX commented 1 month ago

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