From 99e8105624254d7684f637bc66c6c94c213c1989 Mon Sep 17 00:00:00 2001 From: ahh Date: Thu, 6 Dec 2018 13:31:42 -0500 Subject: [PATCH] refactor: restructure option handling code --- cmd/depviz/cmd_airtable.go | 53 ++++++++++++++++++------------- cmd/depviz/cmd_db.go | 37 +++++++++++++--------- cmd/depviz/cmd_graph.go | 43 +++++++++++++++---------- cmd/depviz/cmd_pull.go | 31 +++++++++++------- cmd/depviz/cmd_run.go | 39 ++++++++++++++--------- cmd/depviz/cmd_web.go | 31 +++++++++++------- cmd/depviz/main.go | 64 ++++++++++++++++++-------------------- 7 files changed, 175 insertions(+), 123 deletions(-) diff --git a/cmd/depviz/cmd_airtable.go b/cmd/depviz/cmd_airtable.go index 5d8b0cea0..85431d4c6 100644 --- a/cmd/depviz/cmd_airtable.go +++ b/cmd/depviz/cmd_airtable.go @@ -28,39 +28,48 @@ type airtableOptions struct { Targets []repo.Target `mapstructure:"targets"` } -var globalAirtableOptions airtableOptions - func (opts airtableOptions) String() string { out, _ := json.Marshal(opts) return string(out) } -func airtableSetupFlags(flags *pflag.FlagSet, opts *airtableOptions) { - flags.StringVarP(&opts.IssuesTableName, "airtable-issues-table-name", "", "Issues and PRs", "Airtable issues table name") - flags.StringVarP(&opts.RepositoriesTableName, "airtable-repositories-table-name", "", "Repositories", "Airtable repositories table name") - flags.StringVarP(&opts.AccountsTableName, "airtable-accounts-table-name", "", "Accounts", "Airtable accounts table name") - flags.StringVarP(&opts.LabelsTableName, "airtable-labels-table-name", "", "Labels", "Airtable labels table name") - flags.StringVarP(&opts.MilestonesTableName, "airtable-milestones-table-name", "", "Milestones", "Airtable milestones table nfame") - flags.StringVarP(&opts.ProvidersTableName, "airtable-providers-table-name", "", "Providers", "Airtable providers table name") - flags.StringVarP(&opts.BaseID, "airtable-base-id", "", "", "Airtable base ID") - flags.StringVarP(&opts.Token, "airtable-token", "", "", "Airtable token") - flags.BoolVarP(&opts.DestroyInvalidRecords, "airtable-destroy-invalid-records", "", false, "Destroy invalid records") +type airtableCommand struct { + opts airtableOptions +} + +func (cmd *airtableCommand) LoadDefaultOptions() error { + if err := viper.Unmarshal(&cmd.opts); err != nil { + return err + } + return nil +} + +func (cmd *airtableCommand) ParseFlags(flags *pflag.FlagSet) { + flags.StringVarP(&cmd.opts.IssuesTableName, "airtable-issues-table-name", "", "Issues and PRs", "Airtable issues table name") + flags.StringVarP(&cmd.opts.RepositoriesTableName, "airtable-repositories-table-name", "", "Repositories", "Airtable repositories table name") + flags.StringVarP(&cmd.opts.AccountsTableName, "airtable-accounts-table-name", "", "Accounts", "Airtable accounts table name") + flags.StringVarP(&cmd.opts.LabelsTableName, "airtable-labels-table-name", "", "Labels", "Airtable labels table name") + flags.StringVarP(&cmd.opts.MilestonesTableName, "airtable-milestones-table-name", "", "Milestones", "Airtable milestones table nfame") + flags.StringVarP(&cmd.opts.ProvidersTableName, "airtable-providers-table-name", "", "Providers", "Airtable providers table name") + flags.StringVarP(&cmd.opts.BaseID, "airtable-base-id", "", "", "Airtable base ID") + flags.StringVarP(&cmd.opts.Token, "airtable-token", "", "", "Airtable token") + flags.BoolVarP(&cmd.opts.DestroyInvalidRecords, "airtable-destroy-invalid-records", "", false, "Destroy invalid records") viper.BindPFlags(flags) } -func newAirtableCommand() *cobra.Command { - cmd := &cobra.Command{ +func (cmd *airtableCommand) NewCobraCommand(dc map[string]DepvizCommand) *cobra.Command { + cc := &cobra.Command{ Use: "airtable", } - cmd.AddCommand(newAirtableSyncCommand()) - return cmd + cc.AddCommand(cmd.airtableSyncCommand()) + return cc } -func newAirtableSyncCommand() *cobra.Command { - cmd := &cobra.Command{ +func (cmd *airtableCommand) airtableSyncCommand() *cobra.Command { + cc := &cobra.Command{ Use: "sync", - RunE: func(cmd *cobra.Command, args []string) error { - opts := globalAirtableOptions + RunE: func(_ *cobra.Command, args []string) error { + opts := cmd.opts var err error if opts.Targets, err = repo.ParseTargets(args); err != nil { return errors.Wrap(err, "invalid targets") @@ -68,8 +77,8 @@ func newAirtableSyncCommand() *cobra.Command { return airtableSync(&opts) }, } - airtableSetupFlags(cmd.Flags(), &globalAirtableOptions) - return cmd + cmd.ParseFlags(cc.Flags()) + return cc } // TODO: Make this function a lot shorter by pulling out some of the boilerplate? diff --git a/cmd/depviz/cmd_db.go b/cmd/depviz/cmd_db.go index c318722b3..825ce471c 100644 --- a/cmd/depviz/cmd_db.go +++ b/cmd/depviz/cmd_db.go @@ -12,35 +12,44 @@ import ( type dbOptions struct{} -var globalDBOptions dbOptions - func (opts dbOptions) String() string { out, _ := json.Marshal(opts) return string(out) } -func dbSetupFlags(flags *pflag.FlagSet, opts *dbOptions) { - viper.BindPFlags(flags) +type dbCommand struct { + opts dbOptions +} + +func (cmd *dbCommand) LoadDefaultOptions() error { + if err := viper.Unmarshal(&cmd.opts); err != nil { + return err + } + return nil } -func newDBCommand() *cobra.Command { - cmd := &cobra.Command{ +func (cmd *dbCommand) NewCobraCommand(dc map[string]DepvizCommand) *cobra.Command { + cc := &cobra.Command{ Use: "db", } - cmd.AddCommand(newDBDumpCommand()) - return cmd + cc.AddCommand(cmd.dbDumpCommand()) + return cc +} + +func (cmd *dbCommand) ParseFlags(flags *pflag.FlagSet) { + viper.BindPFlags(flags) } -func newDBDumpCommand() *cobra.Command { - cmd := &cobra.Command{ +func (cmd *dbCommand) dbDumpCommand() *cobra.Command { + cc := &cobra.Command{ Use: "dump", - RunE: func(cmd *cobra.Command, args []string) error { - opts := globalDBOptions + RunE: func(_ *cobra.Command, args []string) error { + opts := cmd.opts return dbDump(&opts) }, } - dbSetupFlags(cmd.Flags(), &globalDBOptions) - return cmd + cmd.ParseFlags(cc.Flags()) + return cc } func dbDump(opts *dbOptions) error { diff --git a/cmd/depviz/cmd_graph.go b/cmd/depviz/cmd_graph.go index 9b3f6eba0..cd2becbe1 100644 --- a/cmd/depviz/cmd_graph.go +++ b/cmd/depviz/cmd_graph.go @@ -34,31 +34,40 @@ type graphOptions struct { // NoExternal } -var globalGraphOptions graphOptions - func (opts graphOptions) String() string { out, _ := json.Marshal(opts) return string(out) } -func graphSetupFlags(flags *pflag.FlagSet, opts *graphOptions) { - flags.BoolVarP(&opts.ShowClosed, "show-closed", "", false, "show closed issues") - flags.BoolVarP(&opts.DebugGraph, "debug-graph", "", false, "debug graph") - flags.BoolVarP(&opts.ShowOrphans, "show-orphans", "", false, "show issues not linked to an epic") - flags.BoolVarP(&opts.NoCompress, "no-compress", "", false, "do not compress graph (no overlap)") - flags.BoolVarP(&opts.DarkTheme, "dark-theme", "", false, "dark theme") - flags.BoolVarP(&opts.ShowPRs, "show-prs", "", false, "show PRs") - flags.StringVarP(&opts.Output, "output", "o", "-", "output file ('-' for stdout, dot)") - flags.StringVarP(&opts.Format, "format", "f", "", "output file format (if empty, will determine thanks to output extension)") +type graphCommand struct { + opts graphOptions +} + +func (cmd *graphCommand) LoadDefaultOptions() error { + if err := viper.Unmarshal(&cmd.opts); err != nil { + return err + } + return nil +} + +func (cmd *graphCommand) ParseFlags(flags *pflag.FlagSet) { + flags.BoolVarP(&cmd.opts.ShowClosed, "show-closed", "", false, "show closed issues") + flags.BoolVarP(&cmd.opts.DebugGraph, "debug-graph", "", false, "debug graph") + flags.BoolVarP(&cmd.opts.ShowOrphans, "show-orphans", "", false, "show issues not linked to an epic") + flags.BoolVarP(&cmd.opts.NoCompress, "no-compress", "", false, "do not compress graph (no overlap)") + flags.BoolVarP(&cmd.opts.DarkTheme, "dark-theme", "", false, "dark theme") + flags.BoolVarP(&cmd.opts.ShowPRs, "show-prs", "", false, "show PRs") + flags.StringVarP(&cmd.opts.Output, "output", "o", "-", "output file ('-' for stdout, dot)") + flags.StringVarP(&cmd.opts.Format, "format", "f", "", "output file format (if empty, will determine thanks to output extension)") //flags.BoolVarP(&opts.Preview, "preview", "p", false, "preview result") viper.BindPFlags(flags) } -func newGraphCommand() *cobra.Command { - cmd := &cobra.Command{ +func (cmd *graphCommand) NewCobraCommand(dc map[string]DepvizCommand) *cobra.Command { + cc := &cobra.Command{ Use: "graph", - RunE: func(cmd *cobra.Command, args []string) error { - opts := globalGraphOptions + RunE: func(_ *cobra.Command, args []string) error { + opts := cmd.opts var err error if opts.Targets, err = repo.ParseTargets(args); err != nil { return errors.Wrap(err, "invalid targets") @@ -66,8 +75,8 @@ func newGraphCommand() *cobra.Command { return graph(&opts) }, } - graphSetupFlags(cmd.Flags(), &globalGraphOptions) - return cmd + cmd.ParseFlags(cc.Flags()) + return cc } func graph(opts *graphOptions) error { diff --git a/cmd/depviz/cmd_pull.go b/cmd/depviz/cmd_pull.go index d1a5bc02e..8ca65aaef 100644 --- a/cmd/depviz/cmd_pull.go +++ b/cmd/depviz/cmd_pull.go @@ -22,24 +22,33 @@ type pullOptions struct { Targets repo.Targets `mapstructure:"targets"` } -var globalPullOptions pullOptions - func (opts pullOptions) String() string { out, _ := json.Marshal(opts) return string(out) } -func pullSetupFlags(flags *pflag.FlagSet, opts *pullOptions) { - flags.StringVarP(&opts.GithubToken, "github-token", "", "", "GitHub Token with 'issues' access") - flags.StringVarP(&opts.GitlabToken, "gitlab-token", "", "", "GitLab Token with 'issues' access") +type pullCommand struct { + opts pullOptions +} + +func (cmd *pullCommand) LoadDefaultOptions() error { + if err := viper.Unmarshal(&cmd.opts); err != nil { + return err + } + return nil +} + +func (cmd *pullCommand) ParseFlags(flags *pflag.FlagSet) { + flags.StringVarP(&cmd.opts.GithubToken, "github-token", "", "", "GitHub Token with 'issues' access") + flags.StringVarP(&cmd.opts.GitlabToken, "gitlab-token", "", "", "GitLab Token with 'issues' access") viper.BindPFlags(flags) } -func newPullCommand() *cobra.Command { - cmd := &cobra.Command{ +func (cmd *pullCommand) NewCobraCommand(dc map[string]DepvizCommand) *cobra.Command { + cc := &cobra.Command{ Use: "pull", - RunE: func(cmd *cobra.Command, args []string) error { - opts := globalPullOptions + RunE: func(_ *cobra.Command, args []string) error { + opts := cmd.opts var err error if opts.Targets, err = repo.ParseTargets(args); err != nil { return errors.Wrap(err, "invalid targets") @@ -47,8 +56,8 @@ func newPullCommand() *cobra.Command { return pullAndCompute(&opts) }, } - pullSetupFlags(cmd.Flags(), &globalPullOptions) - return cmd + cmd.ParseFlags(cc.Flags()) + return cc } func pullAndCompute(opts *pullOptions) error { diff --git a/cmd/depviz/cmd_run.go b/cmd/depviz/cmd_run.go index a28d98792..6ae3f79aa 100644 --- a/cmd/depviz/cmd_run.go +++ b/cmd/depviz/cmd_run.go @@ -17,26 +17,35 @@ type runOptions struct { NoPull bool `mapstructure:"no-pull"` } -var globalRunOptions runOptions - func (opts runOptions) String() string { out, _ := json.Marshal(opts) return string(out) } -func runSetupFlags(flags *pflag.FlagSet, opts *runOptions) { - flags.BoolVarP(&opts.NoPull, "no-pull", "", false, "do not pull new issues before running") - flags.StringSliceVarP(&opts.AdditionalPulls, "additional-pulls", "", []string{}, "additional pull that won't necessarily be displayed on the graph") +type runCommand struct { + opts runOptions +} + +func (cmd *runCommand) LoadDefaultOptions() error { + if err := viper.Unmarshal(&cmd.opts); err != nil { + return err + } + return nil +} + +func (cmd *runCommand) ParseFlags(flags *pflag.FlagSet) { + flags.BoolVarP(&cmd.opts.NoPull, "no-pull", "", false, "do not pull new issues before running") + flags.StringSliceVarP(&cmd.opts.AdditionalPulls, "additional-pulls", "", []string{}, "additional pull that won't necessarily be displayed on the graph") viper.BindPFlags(flags) } -func newRunCommand() *cobra.Command { - cmd := &cobra.Command{ +func (cmd *runCommand) NewCobraCommand(dc map[string]DepvizCommand) *cobra.Command { + cc := &cobra.Command{ Use: "run", - RunE: func(cmd *cobra.Command, args []string) error { - opts := globalRunOptions - opts.GraphOptions = globalGraphOptions - opts.PullOptions = globalPullOptions + RunE: func(_ *cobra.Command, args []string) error { + opts := cmd.opts + opts.GraphOptions = dc["graph"].(*graphCommand).opts + opts.PullOptions = dc["pull"].(*pullCommand).opts targets, err := repo.ParseTargets(args) if err != nil { @@ -51,10 +60,10 @@ func newRunCommand() *cobra.Command { return run(&opts) }, } - runSetupFlags(cmd.Flags(), &globalRunOptions) - graphSetupFlags(cmd.Flags(), &globalGraphOptions) - pullSetupFlags(cmd.Flags(), &globalPullOptions) - return cmd + cmd.ParseFlags(cc.Flags()) + dc["graph"].ParseFlags(cc.Flags()) + dc["pull"].ParseFlags(cc.Flags()) + return cc } func run(opts *runOptions) error { diff --git a/cmd/depviz/cmd_web.go b/cmd/depviz/cmd_web.go index 4bc9ce250..6fa7b937c 100644 --- a/cmd/depviz/cmd_web.go +++ b/cmd/depviz/cmd_web.go @@ -28,29 +28,38 @@ type webOptions struct { ShowRoutes bool } -var globalWebOptions webOptions - func (opts webOptions) String() string { out, _ := json.Marshal(opts) return string(out) } -func webSetupFlags(flags *pflag.FlagSet, opts *webOptions) { - flags.StringVarP(&opts.Bind, "bind", "b", ":2020", "web server bind address") - flags.BoolVarP(&opts.ShowRoutes, "show-routes", "", false, "display available routes and quit") +type webCommand struct { + opts webOptions +} + +func (cmd *webCommand) LoadDefaultOptions() error { + if err := viper.Unmarshal(&cmd.opts); err != nil { + return err + } + return nil +} + +func (cmd *webCommand) ParseFlags(flags *pflag.FlagSet) { + flags.StringVarP(&cmd.opts.Bind, "bind", "b", ":2020", "web server bind address") + flags.BoolVarP(&cmd.opts.ShowRoutes, "show-routes", "", false, "display available routes and quit") viper.BindPFlags(flags) } -func newWebCommand() *cobra.Command { - cmd := &cobra.Command{ +func (cmd *webCommand) NewCobraCommand(dc map[string]DepvizCommand) *cobra.Command { + cc := &cobra.Command{ Use: "web", - RunE: func(cmd *cobra.Command, args []string) error { - opts := globalWebOptions + RunE: func(_ *cobra.Command, args []string) error { + opts := cmd.opts return web(&opts) }, } - webSetupFlags(cmd.Flags(), &globalWebOptions) - return cmd + cmd.ParseFlags(cc.Flags()) + return cc } func webListIssues(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/depviz/main.go b/cmd/depviz/main.go index d271262cf..6cb431880 100644 --- a/cmd/depviz/main.go +++ b/cmd/depviz/main.go @@ -11,6 +11,7 @@ import ( _ "github.com/mattn/go-sqlite3" "github.com/pkg/errors" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/spf13/viper" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -34,16 +35,32 @@ var ( db *gorm.DB ) +type DepvizCommand interface { + NewCobraCommand(map[string]DepvizCommand) *cobra.Command + LoadDefaultOptions() error + ParseFlags(*pflag.FlagSet) +} + func newRootCommand() *cobra.Command { - cmd := &cobra.Command{ + rootCmd := &cobra.Command{ Use: "depviz", } - cmd.PersistentFlags().BoolP("help", "h", false, "print usage") - cmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "verbose mode") - cmd.PersistentFlags().StringVarP(&cfgFile, "config", "c", "", "config file (default is ./.depviz.yml)") - cmd.PersistentFlags().StringVarP(&dbPath, "db-path", "", "$HOME/.depviz.db", "database path") + rootCmd.PersistentFlags().BoolP("help", "h", false, "print usage") + rootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "verbose mode") + rootCmd.PersistentFlags().StringVarP(&cfgFile, "config", "c", "", "config file (default is ./.depviz.yml)") + rootCmd.PersistentFlags().StringVarP(&dbPath, "db-path", "", "$HOME/.depviz.db", "database path") - cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + // Add commands here. + cmds := map[string]DepvizCommand { + "pull": &pullCommand{}, + "db": &dbCommand{}, + "airtable": &airtableCommand{}, + "graph": &graphCommand{}, + "run": &runCommand{}, + "web": &webCommand{}, + } + + rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { // configure zap config := zap.NewDevelopmentConfig() if verbose { @@ -72,24 +89,10 @@ func newRootCommand() *cobra.Command { } } - // fill global options - if err := viper.Unmarshal(&globalGraphOptions); err != nil { - return err - } - if err := viper.Unmarshal(&globalRunOptions); err != nil { - return err - } - if err := viper.Unmarshal(&globalPullOptions); err != nil { - return err - } - if err := viper.Unmarshal(&globalWebOptions); err != nil { - return err - } - if err := viper.Unmarshal(&globalAirtableOptions); err != nil { - return err - } - if err := viper.Unmarshal(&globalDBOptions); err != nil { - return err + for _, cmd := range cmds { + if err := cmd.LoadDefaultOptions(); err != nil { + return err + } } // configure sql @@ -121,15 +124,10 @@ func newRootCommand() *cobra.Command { return nil } - cmd.AddCommand( - newPullCommand(), - newDBCommand(), - newAirtableCommand(), - newGraphCommand(), - newRunCommand(), - newWebCommand(), - ) + for _, cmd := range cmds { + rootCmd.AddCommand(cmd.NewCobraCommand(cmds)) + } viper.AutomaticEnv() viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) - return cmd + return rootCmd }