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.

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

Adjust doctor command format and options #21790

Closed
wants to merge 7 commits into from

Conversation

zeripath
Copy link
Contributor

The gitea doctor command is a little weird especially since the recreate-table subcommand.

This PR changes the doctor command to be an empty subcommand and adds gitea doctor check subcommand.

In this subcommand the requested checks are provided as an argument rather than as options. The logging is also changed so that the doctor.log is not provided by default - instead that will only be created if you set --log-file option. --verbose is provided to output this logging to stdout.

gitea convert is also moved to be a subcommand as gitea doctor convert.

Signed-off-by: Andrew Thornton art27@cantab.net

The gitea doctor command is a little weird especially since the
recreate-table subcommand.

This PR changes the doctor command to be an empty subcommand and adds
`gitea doctor check` subcommand.

In this subcommand the requested checks are provided as an argument
rather than as options.  The logging is also changed so that the
doctor.log is not provided by default - instead that will only be
created if you set `--log-file` option. `--verbose` is provided to
output this logging to stdout.

`gitea convert` is also moved to be a subcommand as `gitea doctor convert`.

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added the type/enhancement An improvement of existing functionality label Nov 12, 2022
@zeripath zeripath added this to the 1.19.0 milestone Nov 12, 2022
Signed-off-by: Andrew Thornton <art27@cantab.net>
cmd/doctor.go Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 15, 2022
@lunny lunny added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Feb 3, 2023
@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 Feb 6, 2023
@yardenshoham yardenshoham modified the milestones: 1.19.0, 1.20.0 Feb 22, 2023
@silverwind
Copy link
Member

Need to update version in references.

@delvh delvh removed this from the 1.20.0 milestone Jun 7, 2023
@denyskon
Copy link
Member

@zeripath I briefly looked through the PR, and it looks good to me. Could you please change the version numbers and resolve merge conflicts, so we can finally have this merged?

@denyskon denyskon added this to the 1.21.0 milestone Jul 10, 2023
@denyskon denyskon added issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin labels Jul 10, 2023
@silverwind
Copy link
Member

I guess @zeripath is currently inactive, but you could raise another PR in place of this one.

@wxiaoguang
Copy link
Contributor

There are some problems for this PR:

  • The func init() seems unnecessary, in most cases the Gitea instance doesn't need to print that message, so it wastes CPU and memory
  • The loop for i, name := range names is fragile. I can understand the purpose is to make the command like "./gitea doctor check list default", but, it introduces strange syntax like "./gitea doctor check all default list"

So I proposed a new one: Improve "gitea doctor" sub-command and fix "help" commands #26072

@wxiaoguang wxiaoguang closed this Jul 23, 2023
@GiteaBot GiteaBot removed this from the 1.21.0 milestone Jul 24, 2023
wxiaoguang added a commit that referenced this pull request Jul 25, 2023
Replace #21790

And close #25965 by the way (it needs a separate fix for 1.20)

Major changes:

1. Move "gitea convert" to "gitea doctor conver". The old "gitea doctor"
still works as a hidden sub-command (to avoid breaking)
2. Do not write "doctor.log" by default, it's not useful in most cases
and causes bugs like 25965
3. Improve documents
4. Fix the "help" commands. Before, the "./gitea doctor" can't show the
sub-command help correctly (regression of the last cli/v2 refactoring)

After this PR:

```
./gitea help # show all sub-commands for the app
./gitea doctor # show the sub-commands for the "doctor"
./gitea doctor help # show the sub-commands for the "doctor", as above
```
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Oct 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants