Skip to content

refactor: remove redundant else branches #344

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.

Sign up for GitHub

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

Open
tenthirtyam wants to merge 1 commit into
base: main
Choose a base branch
from
+25 −25
Open

refactor: remove redundant else branches #344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions acctest/pluginacc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ func TestPlugin(t *testing.T, testCase *PluginTestCase) {
"acceptance test template can be found at %s",
err.Error(), filepath.Join(cwd, initLogfile),
filepath.Join(cwd, templatePath))
} else {
os.Remove(initLogfile)
}

os.Remove(initLogfile)
}
}

Expand Down Expand Up @@ -156,8 +156,8 @@ func TestPlugin(t *testing.T, testCase *PluginTestCase) {
"acceptance test template can be found at %s",
checkErr.Error(), filepath.Join(cwd, logfile),
filepath.Join(cwd, templatePath))
} else {
os.Remove(templatePath)
os.Remove(logfile)
}

os.Remove(templatePath)
os.Remove(logfile)

@tanmay-hc tanmay-hc Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel we would like to keep the two temporary files in case of test failures as they could help the users debug the failed test better.

Same goes for the change in acctest/provisioneracc/provisioners.go

@tenthirtyam tenthirtyam Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the intent to keep the template and log on failure for debugging (the Fatalf message points at those paths for that reason).

Behaviorally this is unchanged: t.Fatalf never returns (FailNow / runtime.Goexit), so the os.Remove calls after the if only run on success. Same for provisioners.go. Dropping the else after a terminating statement is the usual Go early-exit idiom.

Happy to restore the else in both places if you prefer that form to make the “cleanup only on success” intent more obvious at a glance.

}
6 changes: 3 additions & 3 deletions acctest/provisioneracc/provisioners.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,10 @@ func TestProvisionersAgainstBuilders(testCase *ProvisionerTestCase, t *testing.T
"acceptance test template can be found at %s",
checkErr.Error(), filepath.Join(cwd, logfile),
filepath.Join(cwd, templatePath))
} else {
os.Remove(templatePath)
os.Remove(logfile)
}

os.Remove(templatePath)
os.Remove(logfile)
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions multistep/commonsteps/multistep_runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ func newRunner(steps []multistep.Step, config common.PackerConfig, ui packersdk.
if config.PackerDebug {
pauseFn := MultistepDebugFn(ui)
return &multistep.DebugRunner{Steps: steps, PauseFn: pauseFn}, pauseFn
} else {
return &multistep.BasicRunner{Steps: steps}, nil
}

return &multistep.BasicRunner{Steps: steps}, nil
}

// NewRunner returns a multistep.Runner that runs steps augmented with support
Expand Down
4 changes: 2 additions & 2 deletions rpc/build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ func (b *testBuild) Run(ctx context.Context, ui packersdk.Ui) ([]packersdk.Artif

if b.errRunResult {
return nil, errors.New("foo")
} else {
return []packersdk.Artifact{testBuildArtifact}, nil
}

return []packersdk.Artifact{testBuildArtifact}, nil
}

func (b *testBuild) SetDebug(bool) {
Expand Down
18 changes: 9 additions & 9 deletions sdk-internals/communicator/ssh/communicator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,18 +179,18 @@ func (c *comm) Start(ctx context.Context, cmd *packersdk.RemoteCmd) (err error)
func (c *comm) Upload(path string, input io.Reader, fi *os.FileInfo) error {
if c.config.UseSftp {
return c.sftpUploadSession(path, input, fi)
} else {
return c.scpUploadSession(path, input, fi)
}

return c.scpUploadSession(path, input, fi)
}

func (c *comm) UploadDir(dst string, src string, excl []string) error {
log.Printf("[DEBUG] Upload dir '%s' to '%s'", src, dst)
if c.config.UseSftp {
return c.sftpUploadDirSession(dst, src, excl)
} else {
return c.scpUploadDirSession(dst, src, excl)
}

return c.scpUploadDirSession(dst, src, excl)
}

func (c *comm) DownloadDir(src string, dst string, excl []string) error {
Expand Down Expand Up @@ -288,9 +288,9 @@ func (c *comm) newSession() (session *ssh.Session, err error) {

if c.client == nil {
return nil, errors.New("client not available")
} else {
return c.client.NewSession()
}

return c.client.NewSession()
}

return session, nil
Expand Down Expand Up @@ -695,10 +695,10 @@ func (c *comm) scpUploadDirSession(dst string, src string, excl []string) error
return err
}
return scpUploadDirProtocol(srcBase, w, r, uploadEntries, fi)
} else {
// Trailing slash, so only upload the contents
return uploadEntries()
}

// Trailing slash, so only upload the contents
return uploadEntries()
}

return c.scpSession("scp -rvt "+dst, scpFunc)
Expand Down
4 changes: 2 additions & 2 deletions template/config/custom_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ func TrileanFromString(s string) (Trilean, error) {
return TriUnset, err
} else if b {
return TriTrue, nil
} else {
return TriFalse, nil
}

return TriFalse, nil
}

func TrileanFromBool(b bool) Trilean {
Expand Down
4 changes: 2 additions & 2 deletions template/interpolate/funcs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ func passthroughOrInterpolate(data map[interface{}]interface{}, s string) (strin
// TODO match against an actual string constant
if strings.Contains(hp, packerbuilderdata.PlaceholderMsg) {
return fmt.Sprintf("{{.%s}}", s), nil
} else {
return hp, nil
}

return hp, nil
}
}
return "", fmt.Errorf("loaded data, but couldnt find %s in it.", s)
Expand Down
Loading