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

hotfix for ignored --env flag issue (#269) #727

Merged
merged 4 commits into from Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions pop.go
Expand Up @@ -56,6 +56,12 @@ func DialectSupported(d string) bool {
return false
}

// CanonicalDialect checks if the given synonym (could be a valid dialect too)
// is a registered synonym and returns the canonical dialect for the synonym.
// Otherwise, it returns the lowercase value of the given synonym.
//
// Note that it does not check if the return value is a valid (supported)
// dialect so you need to check it with `DialectSupported()`.
func CanonicalDialect(synonym string) string {
d := strings.ToLower(synonym)
if syn, ok := dialectSynonyms[d]; ok {
Expand Down
16 changes: 15 additions & 1 deletion soda/cmd/root.go
Expand Up @@ -20,8 +20,22 @@ var RootCmd = &cobra.Command{
Short: "A tasty treat for all your database needs",
PersistentPreRun: func(c *cobra.Command, args []string) {
fmt.Printf("pop %s\n\n", Version)

/* NOTE: Do not use c.PersistentFlags. `c` is not always the
RootCmd. The naming is confusing. The meaning of "persistent"
in the `PersistentPreRun` is something like "this function will
be 'sticky' to all subcommands and will run for them. So `c`
can be any subcommands.
However, the meaning of "persistent" in the `PersistentFlags`
is, as the function comment said, "persistent FlagSet
specifically set in the **current command**" so it is sticky
to specific command!

Use c.Flags() or c.Root().Flags() here.
*/

// CLI flag has priority
if !c.PersistentFlags().Changed("env") {
if !c.Flags().Changed("env") {
env = defaults.String(os.Getenv("GO_ENV"), env)
}
// TODO! Only do this when the command needs it.
Expand Down
10 changes: 6 additions & 4 deletions soda/cmd/root_integration_test.go
Expand Up @@ -7,27 +7,29 @@ import (
"github.com/stretchr/testify/require"
)

func Test_RootCmd_NoArg(t *testing.T) {
func Test_RootCmd_Environtment(t *testing.T) {
sio4 marked this conversation as resolved.
Show resolved Hide resolved
oldEnv := os.Getenv("GO_ENV")
defer os.Setenv("GO_ENV", oldEnv)

// Fallback on default env
r := require.New(t)
c := RootCmd
c.SetArgs([]string{})

// Fallback on default env
c.SetArgs([]string{"help"})
stanislas-m marked this conversation as resolved.
Show resolved Hide resolved
err := c.Execute()
r.NoError(err)
r.Equal("development", env)

// Override with GO_ENV
c.SetArgs([]string{})
c.SetArgs([]string{"help"})
stanislas-m marked this conversation as resolved.
Show resolved Hide resolved
os.Setenv("GO_ENV", "test")
err = c.Execute()
r.NoError(err)
r.Equal("test", env)

// CLI flag priority
c.SetArgs([]string{
"help",
"--env",
"production",
})
Expand Down