Skip to content

Fix for #320 #345

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

Merged
merged 1 commit into from
Dec 3, 2016
Merged

Fix for #320 #345

merged 1 commit into from
Dec 3, 2016

Conversation

Bwko
Copy link
Member

@Bwko Bwko commented Dec 3, 2016

Suppress the error when we're removing a file that may not exist

Suppress the error when we're removing a file that may not exist
@tboerger
Copy link
Member

tboerger commented Dec 3, 2016

LGTM

@tboerger tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Dec 3, 2016
@thibaultmeyer
Copy link
Contributor

LGTM

@tboerger tboerger 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 Dec 3, 2016
@andreynering andreynering merged commit dc14d0c into go-gitea:master Dec 3, 2016
@andreynering
Copy link
Contributor

Merged. Remember that in some situations makes sense to print to log or create a notice (there's a notice table on DB). Where the file often doesn't exist it is OK to ignore.

@tboerger
Copy link
Member

tboerger commented Dec 3, 2016

To gather more errors and to make errcheck happy we added that, but in this case it fails unintentionally

@lunny lunny added this to the 1.0.0 milestone Dec 4, 2016
@bkcsoft
Copy link
Member

bkcsoft commented Dec 4, 2016

We should probably use log.Infof instead here...

@Bwko
Copy link
Member Author

Bwko commented Dec 4, 2016

I think suppressing these 2 errors is the best solution. Since 90% of the time os.remove will fail and we don't want to flood the log with x does not exist errors.

@bkcsoft
Copy link
Member

bkcsoft commented Dec 4, 2016

Aah right, nvm then. I didn't read the comments above the commands 😛

@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants