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

Introduce go chi web framework as frontend of macaron, so that we can move routes from macaron to chi step by step #7420

Merged
merged 10 commits into from
Nov 13, 2020

Conversation

lunny
Copy link
Member

@lunny lunny commented Jul 11, 2019

According to #7403, we should chose go-chi as Gitea's backend web framework.

Why we should move away from macaron, see @zeripath 's comment #7403 (comment)

This PR will not finish all the work to remove macaron, but it will introduce go chi as the frontend of macaron. Only most middlewares and serval routes will be handled by go chi, and most routes will still be handled by macaron. To implement that, a go chi middleware is written to delegate the HTTP requests to macaron.

A difference between chi and macaron is macaron will set req.Body back but chi will not after consumed the request. That's why the changes to the test is in this PR.

It's ready to review now.

@stale stale bot added the issue/stale label Sep 9, 2019
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 9, 2019
@stale stale bot removed the issue/stale label Sep 9, 2019
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Sep 9, 2019
@lunny
Copy link
Member Author

lunny commented May 2, 2020

related with #7403

@lunny lunny changed the title WIP: Experiment to convert macaron to chi Introduce go chi web framework as frontend of macaron, so that we can move routes from macaron to chi step by step Nov 12, 2020
@go-gitea go-gitea deleted a comment from stale bot Nov 12, 2020
@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Nov 12, 2020
@lunny lunny added this to the 1.14.0 milestone Nov 12, 2020
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

its gitea_v1.14 time ... lests start with it

@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 Nov 12, 2020
@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 Nov 12, 2020
go.mod Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #7420 (f1648ea) into master (c2e05d9) will increase coverage by 0.02%.
The diff coverage is 39.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7420      +/-   ##
==========================================
+ Coverage   42.17%   42.19%   +0.02%     
==========================================
  Files         695      696       +1     
  Lines       76370    76407      +37     
==========================================
+ Hits        32206    32241      +35     
- Misses      38880    38885       +5     
+ Partials     5284     5281       -3     
Impacted Files Coverage Δ
cmd/web.go 0.00% <0.00%> (ø)
routers/routes/chi.go 38.62% <38.62%> (ø)
modules/public/public.go 45.33% <48.27%> (+1.49%) ⬆️
routers/routes/macaron.go 91.93% <50.00%> (ø)
modules/public/dynamic.go 100.00% <100.00%> (ø)
modules/context/panic.go 0.00% <0.00%> (-57.15%) ⬇️
services/pull/pull.go 41.27% <0.00%> (+0.49%) ⬆️
services/pull/check.go 49.63% <0.00%> (+0.72%) ⬆️
modules/queue/unique_queue_disk_channel.go 55.38% <0.00%> (+1.53%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee7133d...f1648ea. Read the comment docs.

@lunny
Copy link
Member Author

lunny commented Nov 13, 2020

@lafriks done.

@6543 6543 requested a review from lafriks November 13, 2020 01:33
@6543
Copy link
Member

6543 commented Nov 13, 2020

@lunny the chi router do not pass things to our logger :/

@lafriks
Copy link
Member

lafriks commented Nov 13, 2020

🚀

@lafriks
Copy link
Member

lafriks commented Nov 13, 2020

LGTM does not seems to like me :/

@lafriks lafriks merged commit c296f4f into go-gitea:master Nov 13, 2020
@lunny lunny deleted the lunny/chi branch November 13, 2020 13:43
@go-gitea go-gitea locked and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. 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.

5 participants