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

[#1917] Commits view: sort everything in reverse chronological order #2024

Merged
merged 5 commits into from
Aug 7, 2023

Conversation

pratham31012002
Copy link
Contributor

@pratham31012002 pratham31012002 commented Jul 23, 2023

This submission is made to fix #1917.

Proposed Commit Message:

Currently days are sorted in reverse chronological order while commits 
are sorted in chronological order.

Sorting days and commits in the same chronological order seems 
more intuitive.

Let's implement changes such that days and commits are sorted in 
the same chronological order. Sorting by LoC also sorts the commits 
of the day by LoC.

Current: Days are sorted in reverse chronological order while commits are sorted in chronological order.

After fix: Days and commits are sorted in the same chronological order. Sorting by LoC also sorts the commits of the day by LoC.

Illustration:

  1. 2021-08-18 when sort by Time, order Ascending
image
  1. 2021-08-18 when sort by Time, order Descending
image
  1. 2021-08-18 when sort by LoC, order Descending
image
  1. 2021-08-18 when sort by LoC, order Ascending
image

Copy link
Member

@ckcherry23 ckcherry23 left a comment

Choose a reason for hiding this comment

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

Thanks for your first PR @pratham31012002! Code changes LGTM.
Just remember to update the commit message such that each line has not more than 72 characters.

@pratham31012002
Copy link
Contributor Author

Kindly requesting your attention to review this pending PR at your earliest convenience.

@ckcherry23 ckcherry23 requested a review from a team August 1, 2023 16:36
@chan-j-d
Copy link
Contributor

chan-j-d commented Aug 2, 2023

Hi @pratham31012002, apologies as there appears to be a new frontend test zoomView/zoomView_diffstat.cy.js merged after the last CI run that fails. Could you take a look at it?

@pratham31012002
Copy link
Contributor Author

pratham31012002 commented Aug 7, 2023

Hi @pratham31012002, apologies as there appears to be a new frontend test zoomView/zoomView_diffstat.cy.js merged after the last CI run that fails. Could you take a look at it?

Hey I have fixed this test case, Could you have a look?

@chan-j-d
Copy link
Contributor

chan-j-d commented Aug 7, 2023

Thanks @pratham31012002 for fixing it. LGTM!

@chan-j-d chan-j-d merged commit b25d1fe into reposense:master Aug 7, 2023
10 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

The following links are for previewing this pull request:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Commits view: sort everything in reverse chronological order
4 participants