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: added descending sort for commits on contributor section in settings page #363 #365

Merged
merged 4 commits into from
Oct 21, 2024

Conversation

Preston-Cook
Copy link
Contributor

@Preston-Cook Preston-Cook commented Oct 21, 2024

Add descending sort by commits for contributors on the settings page.

Huly®: UTRP-349


This change is Reviewable

@Preston-Cook Preston-Cook changed the title fix: bug fix to address issue #363 fix: added descending sort for commits on contributor section in settings page #363 Oct 21, 2024
@Samathingamajig Samathingamajig self-requested a review October 21, 2024 19:52
Copy link
Collaborator

@Samathingamajig Samathingamajig left a comment

Choose a reason for hiding this comment

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

LGTM

@Samathingamajig
Copy link
Collaborator

at some point we need something better than just pulling from the github API on every settings page open, but that's out of scope for this PR

@Samathingamajig Samathingamajig self-requested a review October 21, 2024 20:01
@Samathingamajig
Copy link
Collaborator

awaiting response from @IsaDavRod for the question

should LONGHORN_DEVELOPERS_SWE also be sorted? currently they are, by coincidence, but in general, should they be sorted by # of commits or just by the order defined in the array?
image

@Samathingamajig Samathingamajig dismissed their stale review October 21, 2024 20:04

awaiting response from isaiah

Copy link
Collaborator

@Samathingamajig Samathingamajig left a comment

Choose a reason for hiding this comment

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

looks good, but can this also sort the LONGHORN_DEVELOPERS_SWE before the map?

The admin section at the top should still be based entirely on the order of the file, and LD SWE's should still be above any non-LD contributor, but the SWEs should also be sorted amongst themselves

@Preston-Cook
Copy link
Contributor Author

Added sorting to LONGHORN_DEVELOPERS_SWE. Please ignore the third commit lol. I was working on another issue and accidentally committed WIP changes

Copy link
Collaborator

@Samathingamajig Samathingamajig left a comment

Choose a reason for hiding this comment

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

LGTM

@Samathingamajig Samathingamajig merged commit a715bbd into Longhorn-Developers:main Oct 21, 2024
12 checks 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