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

Move admin routers from /admin to /-/admin #32189

Merged
merged 7 commits into from
Oct 10, 2024
Merged

Move admin routers from /admin to /-/admin #32189

merged 7 commits into from
Oct 10, 2024

Conversation

lunny
Copy link
Member

@lunny lunny commented Oct 3, 2024

Resolve #32181

@lunny lunny added the type/enhancement An improvement of existing functionality label Oct 3, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 3, 2024
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/frontend labels Oct 3, 2024
@lafriks
Copy link
Member

lafriks commented Oct 4, 2024

Test failures are related

sundaram2021
sundaram2021 reviewed Oct 5, 2024
View reviewed changes
Copy link

@sundaram2021 sundaram2021 left a comment

Choose a reason for hiding this comment

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

review

good to merge
I like the way you did it :)

delvh
delvh approved these changes Oct 5, 2024
View reviewed changes
Copy link
Member

@delvh delvh 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.

Two things:

  1. I hope we didn't overlook any URLs still pointing to the old path. Oh well, there will be bug reports if that happens.
  2. I think this will be one of the largest UI-link breaking actions we have ever done. Let's hope our users muscle memory can be overwritten soon enough (for those that typed the URL themselves instead of finding a link)

@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 Oct 5, 2024
@yp05327

This comment was marked as outdated.

Sign in to view
@lunny
Copy link
Member Author

lunny commented Oct 7, 2024

Seems like a break change?

If the user always uses the menu to enter the admin page, it should not be considered as a break change.

@delvh
Copy link
Member

delvh commented Oct 7, 2024

Technically, changing UI routes is not breaking as it does not break anything for anyone who simply uses the UI as intended.
However, what it does break is the muscle memory of users who typed the link themselves.
That action is not covered by our current definition of breaking.

@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 Oct 10, 2024
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Oct 10, 2024 •
edited
Loading

Conflicts caused by my PR have been resolved 1c601b5909fbedfe4d3d177507cd02aa8c0e0732

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 10, 2024
@lunny lunny enabled auto-merge (squash) October 10, 2024 04:31
@lunny lunny merged commit c221767 into go-gitea:main Oct 10, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.24.0 milestone Oct 10, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 10, 2024
@yp05327 yp05327 modified the milestones: 1.24.0, 1.23.0 Oct 10, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 11, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
58fd331 
* giteaofficial/main:
  Fix checkbox bug on private/archive filter (go-gitea#32236)
  Improve the maintainblity of the reserved username list (go-gitea#32229)
  Move admin routers from /admin to /-/admin (go-gitea#32189)
@lunny lunny deleted the lunny/move_admin_router branch November 7, 2024 06:47
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sundaram2021 sundaram2021 sundaram2021 left review comments

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@delvh delvh delvh approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them modifies/frontend modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.

Allow user name "admin" to be created
7 participants
@lunny @lafriks @yp05327 @delvh @wxiaoguang @sundaram2021 @GiteaBot