Skip to content

Conversation

6543
Copy link
Member

@6543 6543 commented Apr 20, 2022

as title - to prevent "pipe leaks"

@6543 6543 requested a review from zeripath April 20, 2022 22:54
@6543 6543 added the type/bug label Apr 20, 2022
@6543 6543 added this to the 1.17.0 milestone Apr 20, 2022
@6543 6543 changed the title Repo assignment ensure to close before overwrite RepoAssignment ensure to close before overwrite Apr 20, 2022
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 20, 2022
Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

Apart from the two currently open suggestions LGTM

@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 Apr 21, 2022
@6543
Copy link
Member Author

6543 commented Apr 21, 2022

@wxiaoguang added some smal refactoring - I wont backport this but its good to have in main

@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 Apr 21, 2022
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@23d3767). Click here to learn what that means.
The diff coverage is 68.96%.

@@           Coverage Diff           @@
##             main   #19449   +/-   ##
=======================================
  Coverage        ?   47.40%           
=======================================
  Files           ?      951           
  Lines           ?   132339           
  Branches        ?        0           
=======================================
  Hits            ?    62732           
  Misses          ?    62051           
  Partials        ?     7556           
Impacted Files Coverage Δ
modules/context/repo.go 51.77% <22.22%> (ø)
routers/api/v1/repo/notes.go 82.14% <50.00%> (ø)
modules/context/api.go 72.01% <100.00%> (ø)
routers/api/v1/api.go 75.98% <100.00%> (ø)

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 23d3767...f5c5982. Read the comment docs.

@6543 6543 added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Apr 21, 2022
@6543
Copy link
Member Author

6543 commented Apr 21, 2022

current conclusion ... we need a unified way to inject and get gitRepo from context e.g. either use git.RepositoryFromContextOrOpen() or pass gitRepo as argument, whatever simplify the code in the specific case best

@6543
Copy link
Member Author

6543 commented Apr 21, 2022

🚀

@6543 6543 merged commit c764355 into go-gitea:main Apr 21, 2022
@6543 6543 deleted the RepoAssignment-ensure-to-close-before-overwrite branch April 21, 2022 15:18
@6543 6543 added the backport/done All backports for this PR have been created label Apr 21, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 21, 2022
* giteaofficial/main:
  Fix logging of Transfer API (go-gitea#19456)
  RepoAssignment ensure to close before overwrite (go-gitea#19449)
  node12 is EOL (go-gitea#19451)
  Add Changelog v1.16.6 (go-gitea#19339) (go-gitea#19450)
  Fix DELETE request for non-existent public key (go-gitea#19443)
  [skip ci] Updated translations via Crowdin
  Don't panic on `ErrEmailInvalid` (go-gitea#19441)
6543 added a commit that referenced this pull request Apr 21, 2022
* check if GitRepo already open and close if

* Only run RepoAssignment once
6543 added a commit that referenced this pull request Apr 25, 2022
#19461)

as per #19449 (comment)

pass gitRepo down to GetRawDiff, since its used for main repo and wiki
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
* check if GitRepo already open and close if

* only run RepoAssignment once

* refactor context helper for api to open GitRepo
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
go-gitea#19461)

as per go-gitea#19449 (comment)

pass gitRepo down to GetRawDiff, since its used for main repo and wiki
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug 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