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

Restructure JS files #30480

Closed
wants to merge 1 commit into from
Closed

Restructure JS files #30480

wants to merge 1 commit into from

Conversation

silverwind
Copy link
Member

Pure file renames. Now the folders in web_src have no nested folders in it, everything inside those folders is "flat" which I find easier to work with and the amount of files in features is drastically reduced by moving all repo-* files to repo subdirectory.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 14, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 14, 2024
@silverwind silverwind added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Apr 14, 2024
@wxiaoguang
Copy link
Contributor

Please do not do so too early.

It would make backporting much more difficult.

@silverwind
Copy link
Member Author

silverwind commented Apr 14, 2024

Hmm we could postpone this after 1.22 release for when the backport volume has settled. I would also like to do the typescript which is another big upcoming refactor.

@wxiaoguang
Copy link
Contributor

#30165 (comment)

Quote my comment again:

I think it needs a clear roadmap about how to refactor. I ever wrote something in https://docs.gitea.com/contributing/guidelines-refactoring

Non-bug refactoring is preferred to be done at the beginning of a milestone, it would be easier to find problems before the release.

What I meant for at the beginning of a milestone is:

1. It should start at next milestone's beginning when the last release is overall stable (eg: after RC or after the first 1.22.1 or 1.22.2 with bug fixes)

2. It should start before the RC period, to avoid introduce unnecessary regressions.

However, since there are really a lot of frontend refactorings for 1.22 (especially when it was going into freezing time), so I think this time it is still better to merge some PRs back to 1.22, to make the future bug fixes easier to backport.

* Then clearly define 1.22-rc1 as a "stable" RC, don't backport any unnecessary PRs at that time.

@silverwind
Copy link
Member Author

silverwind commented Apr 14, 2024

So eiter we wait with this 1-2 months or do it now and backport it (assuming not too much work). I think I lean towards backporting it. It's completely without risk, eslint catches all import-related errors.

@wxiaoguang
Copy link
Contributor

So eiter we wait with this 1-2 months or do it now and backport it (assuming not too much work). I think I lean towards backporting it. It's completely without risk, eslint catches all import-related errors.

But there are still some frontend refactoring not backported. I don't know how you would clean up these problems.

TBH the 1.22-rc0 release procedure is totally a mess, since it is already a mess, I could be neutral and won't block.

@silverwind
Copy link
Member Author

Let's do this later together with the typescript.

@silverwind silverwind closed this Apr 14, 2024
@silverwind silverwind deleted the rn branch April 14, 2024 18:39
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/docs modifies/js size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants