Skip to content

Sorting: Let's move new pulls to the top #427

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.

Sign up for GitHub

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

Merged
danielbeardsley merged 1 commit into from
Dec 3, 2025
Merged

Sorting: Let's move new pulls to the top #427

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions frontend/src/pulldasher/sort.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ export function defaultCompare(a: Pull, b: Pull): number {
// Pulls I have to CR/QA above those I don't
compareBool(a.hasOutdatedSig(getUser()), b.hasOutdatedSig(getUser())) ||
// Pulls I haven't touched vs those I have already CRed
compareBool(!a.hasCurrentSig(getUser()), !b.hasCurrentSig(getUser()))
compareBool(!a.hasCurrentSig(getUser()), !b.hasCurrentSig(getUser())) ||
// younger pulls above older pulls so new things get review sooner
b.created_at.localeCompare(a.created_at)
);
}

Expand All @@ -25,8 +27,8 @@ export function QACompare(a: Pull, b: Pull): number {
compareBool(!a.getLabel("QAing"), !b.getLabel("QAing")) ||
// Pulls with CR completed above those that need more
compareBool(a.isCrDone(), b.isCrDone()) ||
// Older pulls before younger pulls
a.created_at.localeCompare(b.created_at)
// younger pulls before older pulls
b.created_at.localeCompare(a.created_at)
Copy link
Contributor

@jarstelfox jarstelfox Nov 20, 2025

Choose a reason for hiding this comment

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

Should we do updated_at instead? i.e. if someone is doing a good job updating the code? I know that timestamp may count for things like comments right now

Copy link
Member Author

@danielbeardsley danielbeardsley Nov 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Should we do updated_at instead?

That would be the updated_at of the pull request itself (which I think is just bumped when a pull description or title is updated).

It's a good point though. We might be able to get a more change-aware date (most recent comment or something).

Not 100% sure we want that (sort order changing frequently), but I'm open to it.

Copy link
Contributor

@jarstelfox jarstelfox Nov 20, 2025

Choose a reason for hiding this comment

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

The most sane sorting I can think of, related to updates, is the date of the last commit.

I'm not sure I'd want new comments to sort to the top, just new code.

);
}

Expand Down