Reduce cross-package exported variables

It also helps to return errors instead of logging immediately.

This change puts config.Config back in root cmd,
as well as Logger which gets passed around.

client/client.go

@@ -2,8 +2,9 @@ package client
 
 import (
 	"context"
+	"encoding/json"
 
-	"github.com/circleci/circleci-cli/config"
+	"github.com/circleci/circleci-cli/logger"
 	"github.com/machinebox/graphql"
 )
 
@@ -12,29 +13,37 @@ type Client struct {
 	endpoint string
 	token    string
 	client   *graphql.Client
+	logger   *logger.Logger
 }
 
 // NewClient returns a reference to a Client.
 // We also call graphql.NewClient to initialize a new GraphQL Client.
-func NewClient(endpoint string, token string) *Client {
+// Then we pass the Logger originally constructed as cmd.Logger.
+func NewClient(endpoint string, token string, logger *logger.Logger) *Client {
 	return &Client{
 		endpoint,
 		token,
 		graphql.NewClient(endpoint),
+		logger,
 	}
 }
 
 // Run will construct a request using graphql.NewRequest.
 // Then it will execute the given query using graphql.Client.Run.
 // This function will return the unmarshalled response as JSON.
-func (c *Client) Run(query string) (map[string]interface{}, error) {
+func (c *Client) Run(query string) (string, error) {
 	req := graphql.NewRequest(query)
 	req.Header.Set("Authorization", c.token)
 
 	ctx := context.Background()
 	var resp map[string]interface{}
 
-	config.Logger.Debug("Querying %s with:\n\n%s\n\n", c.endpoint, query)
+	c.logger.Debug("Querying %s with:\n\n%s\n\n", c.endpoint, query)
 	err := c.client.Run(ctx, req, &resp)
-	return resp, err
+
+	b, err := json.MarshalIndent(resp, "", "  ")
+	if err != nil {
+		return "", err
+	}
+	return string(b), err
 }

cmd/diagnostic.go

@@ -1,7 +1,8 @@
 package cmd
 
 import (
-	"github.com/circleci/circleci-cli/config"
+	"errors"
+
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 )
@@ -16,17 +17,16 @@ func diagnostic(cmd *cobra.Command, args []string) {
 	endpoint := viper.GetString("endpoint")
 	token := viper.GetString("token")
 
-	config.Logger.Infoln("\n---\nCircleCI CLI Diagnostics\n---\n")
-	config.Logger.Infof("Config found: %v\n", viper.ConfigFileUsed())
+	Logger.Infoln("\n---\nCircleCI CLI Diagnostics\n---\n")
+	Logger.Infof("Config found: %v\n", viper.ConfigFileUsed())
 
-	config.Logger.Infof("GraphQL API endpoint: %s\n", endpoint)
+	Logger.Infof("GraphQL API endpoint: %s\n", endpoint)
 
 	if token == "token" || token == "" {
-		var err error
-		config.Logger.FatalOnError("Please set a token!", err)
+		Logger.FatalOnError("Please set a token!", errors.New(""))
 	} else {
-		config.Logger.Infoln("OK, got a token.")
+		Logger.Infoln("OK, got a token.")
 	}
 
-	config.Logger.Infof("Verbose mode: %v\n", config.Config.Verbose)
+	Logger.Infof("Verbose mode: %v\n", Config.Verbose)
 }

cmd/query.go

@@ -1,12 +1,10 @@
 package cmd
 
 import (
-	"encoding/json"
 	"io/ioutil"
 	"os"
 
 	"github.com/circleci/circleci-cli/client"
-	"github.com/circleci/circleci-cli/config"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 )
@@ -18,15 +16,17 @@ var queryCmd = &cobra.Command{
 }
 
 func query(cmd *cobra.Command, args []string) {
-	client := client.NewClient(viper.GetString("endpoint"), viper.GetString("token"))
+	client := client.NewClient(viper.GetString("endpoint"), viper.GetString("token"), Logger)
 
 	query, err := ioutil.ReadAll(os.Stdin)
-	config.Logger.FatalOnError("Something happened", err)
+	if err != nil {
+		Logger.FatalOnError("Unable to read query", err)
+	}
 
 	resp, err := client.Run(string(query))
-	config.Logger.FatalOnError("Something happend", err)
-	b, err := json.MarshalIndent(resp, "", "  ")
-	config.Logger.FatalOnError("Could not parse graphql response", err)
+	if err != nil {
+		Logger.FatalOnError("Error occurred when running query", err)
+	}
 
-	config.Logger.Infoln(string(b))
+	Logger.Infoln(string(resp))
 }

cmd/root.go

@@ -1,9 +1,11 @@
 package cmd
 
 import (
+	"fmt"
 	"os"
 
 	"github.com/circleci/circleci-cli/config"
+	"github.com/circleci/circleci-cli/logger"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 )
@@ -19,6 +21,17 @@ func Execute() {
 	}
 }
 
+// Logger is exposed here so we can access it from subcommands.
+// This allows us to print to the log at anytime from within the `cmd` package.
+var Logger *logger.Logger
+
+// Config is the current configuration available to all commands in `cmd` package.
+var Config = &config.Config{
+	Verbose:     false,
+	Name:        "cli",
+	DefaultPath: fmt.Sprintf("%s/.circleci/cli.yml", os.Getenv("HOME")),
+}
+
 var rootCmd = &cobra.Command{
 	Use:   "cli",
 	Short: "Use CircleCI from the command line.",
@@ -31,14 +44,22 @@ func addCommands() {
 }
 
 func init() {
-	cobra.OnInitialize(config.Init)
+	cobra.OnInitialize(setup)
 
-	rootCmd.PersistentFlags().BoolVarP(&config.Config.Verbose, "verbose", "v", false, "Enable verbose logging.")
+	rootCmd.PersistentFlags().BoolVarP(&Config.Verbose, "verbose", "v", false, "Enable verbose logging.")
 
-	rootCmd.PersistentFlags().StringVarP(&config.Config.File, "config", "c", "", "config file (default is $HOME/.circleci/cli.yml)")
+	rootCmd.PersistentFlags().StringVarP(&Config.File, "config", "c", "", "config file (default is $HOME/.circleci/cli.yml)")
 	rootCmd.PersistentFlags().StringP("endpoint", "e", "https://circleci.com/graphql", "the endpoint of your CircleCI GraphQL API")
 	rootCmd.PersistentFlags().StringP("token", "t", "", "your token for using CircleCI")
 
-	config.Logger.FatalOnError("Error binding endpoint flag", viper.BindPFlag("endpoint", rootCmd.PersistentFlags().Lookup("endpoint")))
-	config.Logger.FatalOnError("Error binding token flag", viper.BindPFlag("token", rootCmd.PersistentFlags().Lookup("token")))
+	Logger.FatalOnError("Error binding endpoint flag", viper.BindPFlag("endpoint", rootCmd.PersistentFlags().Lookup("endpoint")))
+	Logger.FatalOnError("Error binding token flag", viper.BindPFlag("token", rootCmd.PersistentFlags().Lookup("token")))
+}
+
+func setup() {
+	Logger = logger.NewLogger(Config.Verbose)
+	Logger.FatalOnError(
+		"Failed to setup configuration",
+		Config.Init(),
+	)
 }

config/config.go

@@ -1,56 +1,50 @@
 package config
 
 import (
+	"errors"
 	"fmt"
 	"os"
 
-	"github.com/circleci/circleci-cli/logger"
 	"github.com/spf13/viper"
 )
 
-type config struct {
+type Config struct {
 	Verbose     bool
 	File        string
 	Name        string
 	DefaultPath string
 }
 
-// Logger is exposed here so we can access it from subcommands.
-// Use this to print to the log at anytime.
-var Logger *logger.Logger
-
-// Config is a struct of the current configuration available at runtime.
-var Config = &config{
-	Verbose:     false,
-	Name:        "cli",
-	DefaultPath: fmt.Sprintf("%s/.circleci/cli.yml", os.Getenv("HOME")),
-}
-
 // Init is called on initialize of the root command.
-func Init() {
-	Logger = logger.NewLogger(Config.Verbose)
-	if err := Read(); err == nil {
-		return
+func (c *Config) Init() error {
+	// try to read the config for the user
+	if err := c.read(); err == nil {
+		return err
 	}
 
-	Logger.FatalOnError("Error creating a new config file", Create())
+	// create a new config, prompting the user
+	if err := c.create(); err != nil {
+		return err
+	}
 
-	Config.File = Config.DefaultPath
-	Logger.FatalOnError(
-		"Failed to re-read config after creating a new file",
-		Read(), // reload config after creating it
-	)
+	c.File = c.DefaultPath
+
+	// reload after creating config
+	if err := c.read(); err != nil {
+		return err
+	}
+	return nil
 }
 
-// Read tries to load the config either from Config.DefaultPath or Config.File.
-func Read() (err error) {
-	if Config.File != "" {
-		viper.SetConfigFile(Config.File)
+// read tries to load the config either from Config.defaultPath or Config.file.
+func (c *Config) read() error {
+	if c.File != "" {
+		viper.SetConfigFile(c.File)
 	}
 
 	if viper.ConfigFileUsed() == "" {
 		viper.AddConfigPath("$HOME/.circleci")
-		viper.SetConfigName(Config.Name)
+		viper.SetConfigName(c.Name)
 	}
 
 	// read in environment variables that match
@@ -59,61 +53,60 @@ func Read() (err error) {
 	viper.AutomaticEnv()
 
 	// If a config file is found, read it in.
-	err = viper.ReadInConfig()
+	err := viper.ReadInConfig()
 	return err
 }
 
-// Create will generate a new config, after asking the user for their token.
-func Create() (err error) {
+// create will generate a new config, after asking the user for their token.
+func (c *Config) create() error {
 	// Don't support creating config at --config flag, only default
-	if Config.File != "" {
-		Logger.Debug("Setting up default config at: %v\n", Config.DefaultPath)
+	if c.File != "" {
+		fmt.Printf("Setting up default config at: %v\n", c.DefaultPath)
 	}
 
 	path := fmt.Sprintf("%s/.circleci", os.Getenv("HOME"))
 
-	if _, err = os.Stat(path); os.IsNotExist(err) {
-		Logger.FatalOnError(
-			fmt.Sprintf("Error creating directory: '%s'", path),
-			os.Mkdir(path, 0700),
-		)
+	if _, err := os.Stat(path); os.IsNotExist(err) {
+		if err = os.Mkdir(path, 0700); err != nil {
+			return errors.New(fmt.Sprintf("Error creating directory: '%s'", path))
+		}
 	} else {
-		Logger.FatalOnError(fmt.Sprintf("Error accessing '%s'", path), err)
+		return errors.New(fmt.Sprintf("Error accessing directory: '%s'", path))
 	}
 
 	// Create default config file
-	if _, err = os.Create(Config.DefaultPath); err != nil {
+	if _, err := os.Create(c.DefaultPath); err != nil {
 		return err
 	}
 
 	// open file with read & write
-	file, err := os.OpenFile(Config.DefaultPath, os.O_RDWR, 0600)
+	file, err := os.OpenFile(c.DefaultPath, os.O_RDWR, 0600)
 	if err != nil {
-		Logger.FatalOnError("", err)
+		return err
 	}
 	defer func() {
-		Logger.FatalOnError("Error closing config file", file.Close())
+		file.Close()
 	}()
 
 	// read flag values
 	endpoint := viper.GetString("endpoint")
 	token := viper.GetString("token")
 
 	if token == "token" || token == "" {
-		Logger.Info("Please enter your CircleCI API token: ")
+		fmt.Print("Please enter your CircleCI API token: ")
 		fmt.Scanln(&token)
-		Logger.Infoln("OK.")
+		fmt.Println("OK.")
 	}
 
 	// format input
 	configValues := fmt.Sprintf("endpoint: %v\ntoken: %v\n", endpoint, token)
 
 	// write new config values to file
 	if _, err = file.WriteString(configValues); err != nil {
-		Logger.FatalOnError("", err)
+		return err
 	}
 
-	Logger.Infof("Your configuration has been created in `%v`.\n", Config.DefaultPath)
-	Logger.Infoln("It can edited manually for advanced settings.")
+	fmt.Printf("Your configuration has been created in `%v`.\n", c.DefaultPath)
+	fmt.Println("It can edited manually for advanced settings.")
 	return err
 }