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.

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

Jump to bottom

Get repo list with OrderBy alpha should respect owner too #30784

Merged
merged 5 commits into from
May 6, 2024
Merged

Get repo list with OrderBy alpha should respect owner too #30784

merged 5 commits into from
May 6, 2024

Conversation

6543
Copy link
Member

@6543 6543 commented Apr 30, 2024

instead of:

  • zowner/gcode
  • awesome/nul
  • zowner/nul
  • zowner/zzz

we will get:

  • awesome/nul
  • zowner/gcode
  • zowner/nul
  • zowner/zzz

@6543 6543 added the type/bug label Apr 30, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 30, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 30, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Apr 30, 2024
@6543 6543 mentioned this pull request Apr 30, 2024
Open
1 task
@lunny
Copy link
Member

lunny commented May 2, 2024

Maybe we should have both? For dashaboard repositories, this order is better. But for explore, maybe order by repository name is better?

@silverwind
Copy link
Member

Hmm I think I prefer new order in both cases.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 2, 2024
@6543
Copy link
Member Author

6543 commented May 2, 2024

Maybe we should have both? For dashaboard repositories, this order is better. But for explore, maybe order by repository name is better?

no at the moment it's just random if you want to sort in explore! - and for single owner lists this does not have any effect so we are fine there too

@6543
Copy link
Member Author

6543 commented May 2, 2024

@lunny PS: all other sort types are not affected at all ... so the pull request i cherry picked it form dose not have anything to do with this bugfix. in the end its just a bugfix :)

@6543 6543 requested a review from lunny May 2, 2024 21:21
@lunny lunny mentioned this pull request May 3, 2024
Open
@6543 6543 added this to the 1.23.0 milestone May 6, 2024
@6543 6543 added the backport/v1.22 This PR should be backported to Gitea 1.22 label May 6, 2024
@lunny
Copy link
Member

lunny commented May 6, 2024

What's pages will this change affect? I have tested locally on explore -> repositories -> sort=alphabetically and looks no any affection. Looks like there is a bug here, the code expect alpha but the URL parameter is alphabetically. Can you do a manual test in your local machine to confirm that?

@6543
Copy link
Member Author

6543 commented May 6, 2024 •
edited
Loading

sure

lunny
lunny approved these changes May 6, 2024
View reviewed changes
Copy link
Member

@lunny lunny left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks like this change will only affect the dashboard right sidebar's repositories sequences and related APIs but not other places. But I'm confused as to why there is the sort parameter. Looks like it will only be used by API.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 6, 2024
@6543
Copy link
Member Author

6543 commented May 6, 2024

yes i can confirm, it's only used at the repo search api and the web route for repo search that the dashbord explicite use ...

PS: we might refactor other repo search code paths to use that ... but thats not a bugfix but a refactoring then :)

so i can confirm it

@6543 6543 merged commit 8e8ca6c into go-gitea:main May 6, 2024
26 checks passed
@6543 6543 deleted the RepoOrderBy-respect-owner-too branch May 6, 2024 14:36
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request May 6, 2024
@6543 @GiteaBot
Get repo list with OrderBy alpha should respect owner too (go-gitea#3…
100480c 
…0784)

instead of:
- zowner/gcode
- awesome/nul
- zowner/nul
- zowner/zzz

we will get:
- awesome/nul
- zowner/gcode
- zowner/nul
- zowner/zzz
@GiteaBot GiteaBot mentioned this pull request May 6, 2024
Merged
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label May 6, 2024
lunny pushed a commit that referenced this pull request May 6, 2024
@GiteaBot @6543
Get repo list with OrderBy alpha should respect owner too (#30784) (#…
cfe6779 
…30875)

Backport #30784 by @6543

instead of:
- zowner/gcode
- awesome/nul
- zowner/nul
- zowner/zzz

we will get:
- awesome/nul
- zowner/gcode
- zowner/nul
- zowner/zzz

Co-authored-by: 6543 <6543@obermui.de>
@6543 6543 mentioned this pull request May 6, 2024
Merged
@6543
Copy link
Member Author

6543 commented May 6, 2024

@lunny -> https://github.com/go-gitea/gitea/pull/30876/files#r1591176082

@6543 6543 mentioned this pull request May 6, 2024
Merged
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 7, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
36f7bf4 
* giteaofficial/main:
  Make "sync branch" also sync object format and add tests (go-gitea#30878)
  Make sure git version&feature are always prepared (go-gitea#30877)
  Get repo list with OrderBy alpha should respect owner too (go-gitea#30784)
  Fix some UI problems (dropdown/container) (go-gitea#30849)
  Fix some UI problems (install/checkbox) (go-gitea#30854)
6543 added a commit that referenced this pull request May 7, 2024
@6543
Repository explore alphabetically order respect owner name (#30882)
d410e2a 
similar to #30784 but only for the repo explore page

is covered by #30876 for the main branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lunny lunny lunny approved these changes

@silverwind silverwind silverwind approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created backport/v1.22 This PR should be backported to Gitea 1.22 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. type/bug
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@6543 @lunny @silverwind @GiteaBot