Skip to content

Revert #13857 #14285

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

Closed
wants to merge 2 commits into from
Closed

Revert #13857 #14285

wants to merge 2 commits into from

Conversation

CirnoT
Copy link
Contributor

@CirnoT CirnoT commented Jan 7, 2021

This reverts #13857 and subsequently #14263 with the exception of keeping modified Drone config that performs tests against Windows builds.

The current implementation of #13857 is broken as it creates two separate sessions that fight, overwriting each other and making logging in impossible on any session provider that is not memory.

Fixes #14271

@codecov-io
Copy link

Codecov Report

Merging #14285 (245bc94) into master (c1e30c9) will increase coverage by 0.09%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14285      +/-   ##
==========================================
+ Coverage   41.83%   41.93%   +0.09%     
==========================================
  Files         743      736       -7     
  Lines       79395    79019     -376     
==========================================
- Hits        33212    33133      -79     
+ Misses      40718    40427     -291     
+ Partials     5465     5459       -6     
Impacted Files Coverage Δ
modules/auth/sso/reverseproxy.go 9.52% <0.00%> (ø)
modules/context/panic.go 0.00% <0.00%> (ø)
modules/templates/dynamic.go 56.14% <ø> (-1.22%) ⬇️
modules/auth/sso/basic.go 51.06% <50.00%> (ø)
routers/routes/chi.go 35.18% <62.50%> (-2.62%) ⬇️
modules/auth/sso/sso.go 32.72% <63.63%> (+0.58%) ⬆️
routers/init.go 50.40% <75.00%> (+4.60%) ⬆️
modules/auth/auth.go 64.04% <81.81%> (+2.50%) ⬆️
modules/auth/sso/oauth2.go 46.77% <88.88%> (ø)
modules/auth/sso/session.go 81.81% <100.00%> (ø)
... and 5 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 4614060...245bc94. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 7, 2021
@6543
Copy link
Member

6543 commented Jan 7, 2021

@lunny do how far is chi migration going?

If it's quit finished, I think we dont need to revert, but if it will take still some time we could consider this here as hotfix

@CirnoT
Copy link
Contributor Author

CirnoT commented Jan 8, 2021

This is critical and breaking, it NEEDS to be reverted if fix is not being proposed

6543
6543 approved these changes Jan 8, 2021
@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 Jan 8, 2021
@6543 6543 added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Jan 8, 2021
@6543 6543 self-requested a review January 8, 2021 01:58
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.

.

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.

@CirnoT
Copy link
Contributor Author

CirnoT commented Jan 8, 2021

As you wish

@CirnoT CirnoT closed this Jan 8, 2021
@CirnoT CirnoT deleted the revert-13857 branch January 8, 2021 02:23
@go-gitea go-gitea locked and limited conversation to collaborators Feb 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2FA issues in dev version
4 participants