Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sorting in frontend #95

Merged
merged 1 commit into from
Nov 25, 2024
Merged

Fix sorting in frontend #95

merged 1 commit into from
Nov 25, 2024

Conversation

Sympatron
Copy link
Contributor

No description provided.

@tdittr
Copy link
Collaborator

tdittr commented Nov 18, 2024

Hey, thanks for the contribution!

Could you describe what/how this change fixes the sorting? Are strings now sorted differently?

@Sympatron
Copy link
Contributor Author

Strings should be sorted the same. The problem was that the sort function should only return 0 if both values are equal. Otherwise it should return a positive or negative number depending on the order.
The second problem was that Array.sort does not return a new array, but sorts in-place instead. Array.toSorted is the correct function here.

Everything is sorted descending except for crate names. I think this makes the most sense.

Copy link
Collaborator

@tdittr tdittr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, cool! Thanks for the fix and the explanation.

@tdittr
Copy link
Collaborator

tdittr commented Nov 18, 2024

Hey, I just noticed that I forgot to add a license to the top-level of the project (only had it in the backend). Would you mind rebasing this on top of main?

(The whole code-base is now explicitly licensed under MIT, except for the driver-db contents, which are under CC0, like the original "Awesome Embedded Rust" list)

@Sympatron
Copy link
Contributor Author

Done.

@tdittr
Copy link
Collaborator

tdittr commented Nov 22, 2024

Thank you :)

Looks like the crates.io dump format changed, so CI failed. I merged #99 which should fix this. Can you rebase/merge from main once more? Sorry for the extra steps :(

@tdittr tdittr added this pull request to the merge queue Nov 25, 2024
Merged via the queue into tweedegolf:main with commit 34b2cc9 Nov 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants