Skip to content

Conversation

@sapk
Copy link
Member

@sapk sapk commented Sep 6, 2018

In order to limit concurrent access to database (mostly for SQLite)

Related: #4848 #2040

@lafriks
Copy link
Member

lafriks commented Sep 7, 2018

Git clone tests seems to fail now

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 7, 2018
@lafriks lafriks added the type/enhancement An improvement of existing functionality label Sep 7, 2018
@lafriks lafriks added this to the 1.7.0 milestone Sep 7, 2018
Copy link
Member

Choose a reason for hiding this comment

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

2018

@sapk
Copy link
Member Author

sapk commented Sep 13, 2018

I have not much time until 2 weeks. I will look at tests since I test clone manually and it worked but I must have forget one corner case.

@sapk sapk force-pushed the serv-use-internals branch from 3d2e752 to 3c3ff11 Compare October 23, 2018 13:16
@codecov-io
Copy link

codecov-io commented Oct 23, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@aefeb8c). Click here to learn what that means.
The diff coverage is 37.25%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #4886   +/-   ##
========================================
  Coverage          ?   37.5%           
========================================
  Files             ?     310           
  Lines             ?   45929           
  Branches          ?       0           
========================================
  Hits              ?   17226           
  Misses            ?   26232           
  Partials          ?    2471
Impacted Files Coverage Δ
routers/private/key.go 20% <20%> (ø)
routers/private/internal.go 58.2% <57.44%> (ø)

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 aefeb8c...63e1f3d. Read the comment docs.

@sapk
Copy link
Member Author

sapk commented Oct 23, 2018

I found the stupid mistake I made. 😑
This PR should be good.

@bkcsoft bkcsoft 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 24, 2018
@lunny
Copy link
Member

lunny commented Oct 24, 2018

Both hooks command also need to use api/internal.

@sapk
Copy link
Member Author

sapk commented Oct 24, 2018

@lunny I will have to verify but hook seems to only use const in models and already use private (api/internal).
We could drop https://github.com/go-gitea/gitea/blob/master/cmd/hook.go#L69 that shouldn't be needed anymore but that will be in another PR.

@lunny
Copy link
Member

lunny commented Oct 24, 2018

@sapk Another PR to remove that is good.

@bkcsoft bkcsoft 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 30, 2018
@lunny lunny merged commit 617a243 into go-gitea:master Oct 30, 2018
@sapk sapk deleted the serv-use-internals branch October 30, 2018 07:03
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants