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

Sort branches alphabethical/lastAccess (#6310) #7456

Conversation

pouleyKetchoupp
Copy link

Fixes #6310

Note: These changes are mostly copied from a previous PR #4124 which has been merged and the changes removed later.

Proposed changes

  • Add a configuration setting to choose the ordering criteria of the branches in the dropdown: by date (default) or alphabetically. It affects the branch list in the top bar menu and the repo objects side panel.

Screenshots

New settings sub-menu

image

Test methodology

Manually tested:

  • Check the branches are shown in date order by default in the top bar menu
  • Check the branches are shown in date order by default in the repo objects side panel
  • Change the setting to alphabetical
  • Check the branches are shown in alphabetical order in the top bar menu
  • Check the branches are shown in alphabetical order in the repo objects side panel
  • Change the setting to last access date
  • Check the branches are shown in date order in the top bar menu
  • Check the branches are shown in date order in the repo objects side panel

Test environment(s)

  • GIT 2.24
  • Windows 10

✒️ I contribute this code under The Developer Certificate of Origin.

@codecov
Copy link

codecov bot commented Nov 26, 2019

Codecov Report

Merging #7456 into master will decrease coverage by 0.01%.
The diff coverage is 81.01%.

@@            Coverage Diff             @@
##           master    #7456      +/-   ##
==========================================
- Coverage   48.98%   48.97%   -0.02%     
==========================================
  Files         756      758       +2     
  Lines       55579    55652      +73     
  Branches     7217     7220       +3     
==========================================
+ Hits        27227    27256      +29     
- Misses      26923    26980      +57     
+ Partials     1429     1416      -13
Flag Coverage Δ
#production 37.88% <81.01%> (ø) ⬆️
#tests 97.53% <ø> (ø) ⬆️

@RussKie
Copy link
Member

RussKie commented Nov 27, 2019

Reverting to the original implementation would not be quite correct.
This would be the commit to revert to 9c7c76f.

@drewnoakes could you please clarify what was broken about the feature?

@pouleyKetchoupp
Copy link
Author

@RussKie It seems the feature that was broken was a different one also using some sorting mechanism, and by removing all alphabetical ordering, the simple branch list sorting was removed too, as a side effect.
Could we decide to come back to the simpler, original implementation? What's wrong with it?

@RussKie
Copy link
Member

RussKie commented Nov 27, 2019

It seems the feature that was broken was a different one also using some sorting mechanism,

No, it wasn't.
I have re-written the original feature because it didn't meet a wider set of requirements such as sorting tags.

@RussKie RussKie added the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Jan 12, 2020
@ghost ghost added the 💤 status: no recent activity The issue is stale label Jan 12, 2020
@ghost
Copy link

ghost commented Jan 12, 2020

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 30 days. It will be closed if no further activity occurs.

@pouleyKetchoupp
Copy link
Author

Do you need more feedback from me?

@ghost ghost removed 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity 💤 status: no recent activity The issue is stale labels Jan 12, 2020
@RussKie
Copy link
Member

RussKie commented Jan 13, 2020

As I said earlier the proposed change is not correct, and it won't be accepted in the current state.

@RussKie RussKie added the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Jan 13, 2020
@pouleyKetchoupp
Copy link
Author

This wasn't handled in a professional way. I wasn't given any constructive feedback to address the problem in order to execute the feature in a "correct way" that was never explained.

@ghost ghost removed the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Jan 13, 2020
@RussKie
Copy link
Member

RussKie commented Jan 13, 2020

I'm sorry you feel this way.
However you decided to fix something without discussing it first. I told you that what you did was not correct, and pointed to the correct commit to revert. You ignored it nor tried to understand why it was so.

@RussKie
Copy link
Member

RussKie commented Oct 19, 2020

Fixed in #8427

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.

Sort branches alphabethical/lastAccess was removed
2 participants