Skip to content

Commit ba7d6e9

Browse files
authored
Reduce cognitive complexity (#911)
## Summary 1. Reduce cognitive complexity to 30 2. remove unnecessary nolint comments 3. Correct some comments 4. Remove unused `IsSymlink()` ## How was it tested? Build and test.
1 parent 21778b6 commit ba7d6e9

File tree

8 files changed

+54
-67
lines changed

8 files changed

+54
-67
lines changed

.golangci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ linters-settings:
4141
- name: bool-literal-in-expr
4242
- name: cognitive-complexity
4343
arguments:
44-
- 36 # TODO: gradually reduce it
44+
- 30 # TODO: gradually reduce it
4545
- name: datarace
4646
- name: duplicated-imports
4747
- name: early-return

internal/boxcli/featureflag/feature.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ type feature struct {
1414

1515
var features = map[string]*feature{}
1616

17-
// nolint:unparam
1817
func disabled(name string) *feature {
1918
if features[name] == nil {
2019
features[name] = &feature{name: name}
@@ -23,7 +22,6 @@ func disabled(name string) *feature {
2322
return features[name]
2423
}
2524

26-
// nolint:unparam
2725
func enabled(name string) *feature {
2826
if features[name] == nil {
2927
features[name] = &feature{name: name}

internal/boxcli/generate/devcontainer_util.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type vscode struct {
3333
Extensions []string `json:"extensions"`
3434
}
3535

36-
// Creates a Dockerfile in path and writes devcontainerDockerfile.tmpl's content into it
36+
// CreateDockerfile creates a Dockerfile in path and writes devcontainerDockerfile.tmpl's content into it
3737
func CreateDockerfile(tmplFS embed.FS, path string) error {
3838
// create dockerfile
3939
file, err := os.Create(filepath.Join(path, "Dockerfile"))

internal/fileutil/fileutil.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,6 @@ func IsFile(path string) bool {
3838
return info.Mode().IsRegular()
3939
}
4040

41-
// IsSymlink returns true if the path exists *and* it is a symlink.
42-
//
43-
// It does *not* traverse symbolic links, and returns true even if the symlink
44-
// is broken.
45-
//
46-
// This is a convenience function that coerces errors to false. If it cannot
47-
// read the path for any reason (including a permission error) it returns false.
48-
func IsSymlink(path string) bool {
49-
info, err := os.Lstat(path)
50-
if err != nil {
51-
return false
52-
}
53-
return (info.Mode().Type() & os.ModeSymlink) == os.ModeSymlink
54-
}
55-
5641
func Exists(path string) bool {
5742
err := TryExists(path)
5843
return err == nil

internal/impl/devbox.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -441,21 +441,21 @@ func (d *Devbox) saveCfg() error {
441441
}
442442

443443
func (d *Devbox) Services() (services.Services, error) {
444-
result := services.Services{}
445444
pluginSvcs, err := plugin.GetServices(d.mergedPackages(), d.projectDir)
446445
if err != nil {
447-
return result, err
446+
return nil, err
448447
}
449448

450449
userSvcs := services.FromProcessComposeYaml(d.projectDir)
451450

452-
svcSet, err := lo.Assign(pluginSvcs, userSvcs), nil
453-
451+
svcSet := lo.Assign(pluginSvcs, userSvcs)
454452
keys := make([]string, 0, len(svcSet))
455453
for k := range svcSet {
456454
keys = append(keys, k)
457455
}
458456
slices.Sort(keys)
457+
458+
result := services.Services{}
459459
for _, k := range keys {
460460
result[k] = svcSet[k]
461461
}

internal/impl/shell.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/alessio/shellescape"
1818
"github.com/pkg/errors"
19+
1920
"go.jetpack.io/devbox/internal/debug"
2021
"go.jetpack.io/devbox/internal/nix"
2122
"go.jetpack.io/devbox/internal/xdg"
@@ -40,12 +41,11 @@ const (
4041
shPosix name = "posix"
4142
)
4243

43-
var ErrNoRecognizableShellFound = errors.New(
44-
"SHELL in undefined, and couldn't find any common shells in PATH")
44+
var ErrNoRecognizableShellFound = errors.New("SHELL in undefined, and couldn't find any common shells in PATH")
4545

4646
// TODO consider splitting this struct's functionality so that there is a simpler
4747
// `nix.Shell` that can produce a raw nix shell once again.
48-
//
48+
4949
// DevboxShell configures a user's shell to run in Devbox. Its zero value is a
5050
// fallback shell that launches a regular Nix shell.
5151
type DevboxShell struct {
@@ -179,8 +179,7 @@ func WithHooksFilePath(hooksFilePath string) ShellOption {
179179
}
180180
}
181181

182-
// TODO: Consider removing this once plugins add env vars directly to binaries
183-
// via wrapper scripts.
182+
// TODO: Consider removing this once plugins add env vars directly to binaries via wrapper scripts.
184183
func WithEnvVariables(envVariables map[string]string) ShellOption {
185184
return func(s *DevboxShell) {
186185
s.env = envVariables

internal/services/config.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,28 +7,30 @@ import (
77

88
"github.com/f1bonacc1/process-compose/src/types"
99
"github.com/pkg/errors"
10+
1011
"go.jetpack.io/devbox/internal/cuecfg"
1112
)
1213

1314
func FromProcessComposeYaml(projectDir string) Services {
1415
// TODO need to handle if a filepath is passed in
15-
if processComposeYaml := lookupProcessCompose(projectDir, ""); processComposeYaml != "" {
16-
userSvcs, err := readProcessCompose(processComposeYaml)
17-
if err != nil {
18-
fmt.Fprintf(os.Stderr, "error reading process-compose.yaml: %s, skipping", err)
19-
return nil
20-
}
21-
return userSvcs
16+
processComposeYaml := lookupProcessCompose(projectDir, "")
17+
if processComposeYaml == "" {
18+
return nil
19+
}
20+
userSvcs, err := readProcessCompose(processComposeYaml)
21+
if err != nil {
22+
fmt.Fprintf(os.Stderr, "error reading process-compose.yaml: %s, skipping", err)
23+
return nil
2224
}
23-
return Services{}
25+
return userSvcs
2426
}
2527

2628
func readProcessCompose(path string) (Services, error) {
2729
processCompose := &types.Project{}
2830
services := Services{}
29-
errors := errors.WithStack(cuecfg.ParseFile(path, processCompose))
30-
if errors != nil {
31-
return nil, errors
31+
err := errors.WithStack(cuecfg.ParseFile(path, processCompose))
32+
if err != nil {
33+
return nil, err
3234
}
3335

3436
for name := range processCompose.Processes {

internal/services/status.go

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/fsnotify/fsnotify"
1414
"github.com/pkg/errors"
15+
1516
"go.jetpack.io/devbox/internal/cloud/envir"
1617
)
1718

@@ -44,40 +45,42 @@ func ListenToChanges(ctx context.Context, opts *ListenerOpts) error {
4445
}()
4546

4647
// Start listening for events.
47-
go func() {
48-
for {
49-
select {
50-
case event, ok := <-watcher.Events:
51-
if !ok {
52-
return
53-
}
48+
go listenToEvents(watcher, opts)
5449

55-
// mutagen sync changes show up as create events
56-
if event.Has(fsnotify.Write) || event.Has(fsnotify.Create) {
57-
status, err := readServiceStatus(event.Name)
58-
if err != nil {
59-
fmt.Fprintf(opts.Writer, "Error reading status file: %s\n", err)
60-
continue
61-
}
50+
// We only want events for the specific host.
51+
return errors.WithStack(watcher.Add(filepath.Join(cloudFilePath(opts.ProjectDir), opts.HostID)))
52+
}
6253

63-
status, saveChanges := opts.UpdateFunc(status)
64-
if saveChanges {
65-
if err := writeServiceStatusFile(event.Name, status); err != nil {
66-
fmt.Fprintf(opts.Writer, "Error updating status file: %s\n", err)
67-
}
68-
}
54+
func listenToEvents(watcher *fsnotify.Watcher, opts *ListenerOpts) {
55+
for {
56+
select {
57+
case event, ok := <-watcher.Events:
58+
if !ok {
59+
return
60+
}
61+
62+
// mutagen sync changes show up as create events
63+
if event.Has(fsnotify.Write) || event.Has(fsnotify.Create) {
64+
status, err := readServiceStatus(event.Name)
65+
if err != nil {
66+
fmt.Fprintf(opts.Writer, "Error reading status file: %s\n", err)
67+
continue
6968
}
70-
case err, ok := <-watcher.Errors:
71-
if !ok {
72-
return
69+
70+
status, saveChanges := opts.UpdateFunc(status)
71+
if saveChanges {
72+
if err := writeServiceStatusFile(event.Name, status); err != nil {
73+
fmt.Fprintf(opts.Writer, "Error updating status file: %s\n", err)
74+
}
7375
}
74-
fmt.Fprintf(opts.Writer, "error: %s\n", err)
7576
}
77+
case err, ok := <-watcher.Errors:
78+
if !ok {
79+
return
80+
}
81+
fmt.Fprintf(opts.Writer, "error: %s\n", err)
7682
}
77-
}()
78-
79-
// We only want events for the specific host.
80-
return errors.WithStack(watcher.Add(filepath.Join(cloudFilePath(opts.ProjectDir), opts.HostID)))
83+
}
8184
}
8285

8386
func cloudFilePath(projectDir string) string {

0 commit comments

Comments
 (0)