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.

Sign up for GitHub

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

Jump to bottom

Refactor doctor #12264

Merged
merged 9 commits into from
Dec 2, 2020
Merged

Refactor doctor #12264

merged 9 commits into from
Dec 2, 2020

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Jul 16, 2020 •
edited
Loading

This PR refactors the doctor code to move it to its own module. It changes the functions to use a passed in logger instead of using a results array.

Fix #12706

@zeripath zeripath added pr/wip This PR is not ready for review type/refactoring Existing code has been cleaned up. There should be no new functionality. labels Jul 16, 2020
@stale
Copy link

stale bot commented Sep 15, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Sep 15, 2020
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 15, 2020
@stale stale bot removed the issue/stale label Sep 15, 2020
@zeripath zeripath mentioned this pull request Oct 22, 2020
Merged
@6543
Copy link
Member

6543 commented Oct 22, 2020 •
edited
Loading

so fare so good - I think with this refactor maintaining doctor is easyer ...

EDIT: just noticed some unrelated enhancements at the logger...

@zeripath
Copy link
Contributor Author

They're not quite unrelated ... we need them for this

@zeripath zeripath changed the title WIP/RFC: Refactor doctor Refactor doctor Oct 22, 2020
@zeripath zeripath removed the pr/wip This PR is not ready for review label Oct 22, 2020
@codecov-io
Copy link

codecov-io commented Oct 23, 2020 •
edited
Loading

Codecov Report

Merging #12264 (7aaeaee) into master (253add8) will increase coverage by 0.01%.
The diff coverage is 17.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12264      +/-   ##
==========================================
+ Coverage   42.23%   42.24%   +0.01%     
==========================================
  Files         699      706       +7     
  Lines       76959    77123     +164     
==========================================
+ Hits        32500    32578      +78     
- Misses      39094    39179      +85     
- Partials     5365     5366       +1
Impacted Files Coverage Δ
cmd/doctor.go 0.00% <0.00%> (ø)
modules/doctor/dbconsistency.go 10.81% <10.81%> (ø)
modules/doctor/mergebase.go 11.76% <11.76%> (ø)
modules/doctor/doctor.go 12.24% <12.24%> (ø)
modules/doctor/paths.go 14.49% <14.49%> (ø)
modules/doctor/authorizedkeys.go 14.81% <14.81%> (ø)
modules/doctor/misc.go 30.20% <30.20%> (ø)
modules/doctor/dbversion.go 42.85% <42.85%> (ø)
... 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 253add8...7aaeaee. Read the comment docs.

6543
6543 reviewed Oct 24, 2020
View reviewed changes
modules/doctor/misc.go Outdated Show resolved Hide resolved
@lunny
Copy link
Member

lunny commented Oct 24, 2020

It's better to move all codes about logger to another PR and this one depends that one.

@zeripath
Copy link
Contributor Author

Then we will need to merge those changes first.

The logger changes are entirely required for this to work

@zeripath
Refactor Logger
664511b 
Refactor Logger to make a logger interface and make it possible to
wrap loggers for specific purposes.
@zeripath zeripath mentioned this pull request Oct 24, 2020
Merged
@zeripath
Merge remote-tracking branch 'origin/master' into refactor-logger
045074c
@zeripath
Refactor Doctor
c78c6bd 
    Move the gitea doctor functions into its own module.
    Use a logger for its messages instead of returning a results string[]

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath @6543
Update modules/doctor/misc.go
42a9019 
Co-authored-by: 6543 <6543@obermui.de>
@6543
Copy link
Member

6543 commented Oct 24, 2020

@zeripath do this solve #12706?

@zeripath @6543
Update modules/doctor/misc.go
3c7d0ac 
Co-authored-by: 6543 <6543@obermui.de>
@zeripath
Copy link
Contributor Author

Probably it does solve it yes

@zeripath zeripath added this to the 1.14.0 milestone Oct 31, 2020
@GiteaBot GiteaBot removed the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 5, 2020
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Nov 5, 2020
@6543 6543 mentioned this pull request Nov 9, 2020
Closed
@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 Dec 2, 2020
@lafriks lafriks merged commit 4569339 into go-gitea:master Dec 2, 2020
@zeripath zeripath deleted the refactor-doctor branch December 2, 2020 09:47
@go-gitea go-gitea locked and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@techknowlogick techknowlogick techknowlogick approved these changes

@6543 6543 6543 approved these changes

Assignees
No one assigned
Labels
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
Milestone
1.14.0
Development

Successfully merging this pull request may close these issues.

[Bug] doctor: crash if not able to open doctor.log
7 participants
@zeripath @6543 @codecov-io @lunny @techknowlogick @lafriks @GiteaBot