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

Return nil values when returning an error #2951

Closed
Tracked by #2953
phillebaba opened this issue Aug 30, 2024 · 1 comment · Fixed by #2988
Closed
Tracked by #2953

Return nil values when returning an error #2951

phillebaba opened this issue Aug 30, 2024 · 1 comment · Fixed by #2988
Assignees
Labels
tech-debt 💳 Debt that the team has charged and needs to repay
Milestone

Comments

@phillebaba
Copy link
Member

A good practice in Go when returning errors is to return nil or empty data for all other return values. It makes sure there is no other potential confusion on the calling end that the data returned is valid and could be used.

Here are some examples where we return data and an error.

return objs, fmt.Errorf("failed to unmarshal manifest: %w", err)

return linkedPath, err

return pkg, err

return installedCharts, err

This is slightly related to #2950 and could be done in conjunction with it.

@phillebaba phillebaba added the tech-debt 💳 Debt that the team has charged and needs to repay label Aug 30, 2024
@phillebaba phillebaba mentioned this issue Aug 30, 2024
7 tasks
@mkcp mkcp self-assigned this Sep 6, 2024
@mkcp mkcp mentioned this issue Sep 6, 2024
2 tasks
@mkcp mkcp added this to the v0.40.0 milestone Sep 10, 2024
@mkcp
Copy link
Contributor

mkcp commented Sep 10, 2024

This isn't the most precise methodology but currently my way of automating this is: for each subdirectory in src/ run
ag "return .*\, err" ./src/$SUBDIR
with the goal of finding error returns without a nil/empty value return.

  • api
  • cmd
  • config (no error returns)
  • injector (not golang, as it turns out!)
  • internal
  • test
  • types
pkg
  • pkg (gets it own section)
  • cluster
  • interactive
  • layout
  • lint
  • message
  • pki
  • transform
  • utils
  • variables
  • zoci

While using using --ignore-dir={./src/pkg/packager,./src/internal/packager,./src/extensions}

If anyone has a sharper way of finding these errors ideas are very welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech-debt 💳 Debt that the team has charged and needs to repay
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants