-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Refactor to use urfave/cli/v2 #25959
Conversation
0c57a1a
to
9cd1223
Compare
conf.ForceSMTPS = c.BoolT("force-smtps") | ||
conf.ForceSMTPS = c.Bool("force-smtps") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, didn't the defaults change in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm not mistaken, BoolT
returned true
when no value has been set, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In "v1", there are some buggy behaviors. In "v2", no problem:
package main
import (
"fmt"
"github.com/urfave/cli/v2"
)
func main() {
app := cli.NewApp()
app.Flags = []cli.Flag{
&cli.BoolFlag{
Name: "b",
},
}
app.Action = func(ctx *cli.Context) error {
fmt.Printf("b: %v\n", ctx.Bool("b"))
return nil
}
_ = app.Run([]string{"test", "-b"})
}
Output: b: true
Update: I made a mistake here. Fixed by following PRs.
* giteaoffical/main: (22 commits) Serve pre-defined files in "public", add "security.txt", add CORS header for ".well-known" (go-gitea#25974) Use frontend fetch for branch dropdown component (go-gitea#25719) Remove commit status running and warning from the dashboard repo list (go-gitea#26036) Refactor to use urfave/cli/v2 (go-gitea#25959) Remove commit status running and warning to align GitHub (go-gitea#25839) Fix escape problems in the branch selector (go-gitea#25875) Update README.md to fix the broken link of Hugo (go-gitea#26008) Support copy protected branch from template repository (go-gitea#25889) Update JS dependencies (go-gitea#26025) Reduce margins on admin pages (go-gitea#26026) Actions Artifacts support uploading multiple files and directories (go-gitea#24874) [skip ci] Updated translations via Crowdin Remove redundant "RouteMethods" method (go-gitea#26024) Adding remaining enum for migration repo model type. (go-gitea#26021) RPM Registry: Show zypper commands for SUSE based distros as well (go-gitea#25981) Fix the route for pull-request's authors (go-gitea#26016) Remove nfnt/resize and oliamb/cutter (go-gitea#25999) Correctly refer to dev tags as nightly in the docker docs (go-gitea#26004) Fix env config parsing for "GITEA____APP_NAME" (go-gitea#26001) Add file status for API "Get a single commit from a repository" (go-gitea#16205) (go-gitea#25831) ...
In #25959 I forgot to add default values to some Bool flags (which were BoolT in cli/v1, BoolT means default to be true) This PR adds the default "Value" for them. ``` ./cmd/manager_logging.go: }, cli.BoolTFlag{ ./cmd/manager_logging.go- Name: "rotate, r", ./cmd/manager_logging.go- Usage: "Rotate logs", -- ./cmd/manager_logging.go: }, cli.BoolTFlag{ ./cmd/manager_logging.go- Name: "daily, d", ./cmd/manager_logging.go- Usage: "Rotate logs daily", -- ./cmd/manager_logging.go: }, cli.BoolTFlag{ ./cmd/manager_logging.go- Name: "compress, z", ./cmd/manager_logging.go- Usage: "Compress rotated logs", -- ./cmd/admin.go: cli.BoolTFlag{ ./cmd/admin.go- Name: "force-smtps", ./cmd/admin.go- Usage: "SMTPS is always used on port 465. Set this to force SMTPS on other ports.", -- ./cmd/admin.go: cli.BoolTFlag{ ./cmd/admin.go- Name: "skip-verify", ./cmd/admin.go- Usage: "Skip TLS verify.", -- ./cmd/admin.go: cli.BoolTFlag{ ./cmd/admin.go- Name: "disable-helo", ./cmd/admin.go- Usage: "Disable SMTP helo.", -- ./cmd/admin.go: cli.BoolTFlag{ ./cmd/admin.go- Name: "skip-local-2fa", ./cmd/admin.go- Usage: "Skip 2FA to log on.", -- ./cmd/admin.go: cli.BoolTFlag{ ./cmd/admin.go- Name: "active", ./cmd/admin.go- Usage: "This Authentication Source is Activated.", ```
Replace #10912
And there are many new tests to cover the CLI behavior
There were some concerns about the "option order in hook scripts" (#10912 (comment)), it's not a problem now. Because the hook script uses
/gitea hook --config=/app.ini pre-receive
format. The "config" is a global option, it can appear anywhere.This PR does its best to avoid breaking anything. The major changes are:
gitea
itself won't accept web's options:--install-port
/--pid
/--port
/--quiet
/--verbose
.... They areweb
sub-command's options../gitea web --pid ....
instead./gitea
can still run theweb
sub-command as shorthand, with default options./gitea --sub-opt subcmd
might equal to./gitea subcmd --sub-opt
(well, might not ...)./gitea subcmd --sub-opt
could be used--config
are not affected