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

chore: ensure we return zeroed value when returning errors #2988

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

mkcp
Copy link
Contributor

@mkcp mkcp commented Sep 11, 2024

Description

This PR goes through everything in src (ignoring packager and extensions) and ensures we don't return an error as well as some kind of partial or dirty state from our return vars. Most of these weren't actual vectors for bugs or system instability, it's just good practice. There's some minor cleanups along the way too.

It's worth noting that I used a specific ag command to find error returns (discussed in #2951) so any error variables not starting with err will have been missed in this refactor. I expect that there will be very few of these given how err is the convention.

I left some review notes which I'll highlight in comments for whoever gets eyes on.

Related Issue

Fixes #2951

Checklist before merging

Signed-off-by: Kit Patella <kit@defenseunicorns.com>
@mkcp mkcp self-assigned this Sep 11, 2024
@mkcp mkcp requested review from a team as code owners September 11, 2024 00:08
Copy link

netlify bot commented Sep 11, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 2d71bda
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/66e2099b6bab420009fdf24f

src/pkg/cluster/pvc.go Outdated Show resolved Hide resolved
src/pkg/cluster/zarf.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 22.58065% with 24 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pkg/cluster/zarf.go 28.57% 4 Missing and 1 partial ⚠️
src/pkg/utils/io.go 0.00% 5 Missing ⚠️
src/cmd/tools/helm/load_plugins.go 0.00% 4 Missing ⚠️
src/pkg/cluster/tunnel.go 0.00% 4 Missing ⚠️
src/pkg/utils/yaml.go 0.00% 3 Missing ⚠️
src/pkg/zoci/pull.go 0.00% 3 Missing ⚠️
Files with missing lines Coverage Δ
src/pkg/cluster/pvc.go 75.00% <ø> (ø)
src/pkg/transform/artifact.go 90.90% <100.00%> (ø)
src/pkg/utils/yaml.go 0.00% <0.00%> (ø)
src/pkg/zoci/pull.go 0.00% <0.00%> (ø)
src/cmd/tools/helm/load_plugins.go 0.00% <0.00%> (ø)
src/pkg/cluster/tunnel.go 11.78% <0.00%> (-0.05%) ⬇️
src/pkg/cluster/zarf.go 19.19% <28.57%> (+0.01%) ⬆️
src/pkg/utils/io.go 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

src/cmd/tools/helm/load_plugins.go Outdated Show resolved Hide resolved
src/pkg/cluster/pvc.go Outdated Show resolved Hide resolved
src/pkg/cluster/zarf.go Outdated Show resolved Hide resolved
src/pkg/cluster/zarf.go Outdated Show resolved Hide resolved
src/pkg/cluster/zarf.go Outdated Show resolved Hide resolved
src/pkg/cluster/zarf.go Outdated Show resolved Hide resolved
Signed-off-by: Kit Patella <kit@defenseunicorns.com>
@mkcp mkcp added this pull request to the merge queue Sep 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 11, 2024
@mkcp mkcp added this pull request to the merge queue Sep 11, 2024
Merged via the queue into main with commit e72a273 Sep 11, 2024
27 checks passed
@mkcp mkcp deleted the mkcp/2951-nil-vals branch September 11, 2024 22:38
mjnagel pushed a commit to defenseunicorns/uds-core that referenced this pull request Sep 20, 2024
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| ghcr.io/zarf-dev/packages/init | minor | `v0.39.0` -> `v0.40.1` |
| [zarf-dev/zarf](https://redirect.github.com/zarf-dev/zarf) | minor |
`v0.39.0` -> `v0.40.1` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>zarf-dev/zarf (zarf-dev/zarf)</summary>

###
[`v0.40.1`](https://redirect.github.com/zarf-dev/zarf/releases/tag/v0.40.1)

[Compare
Source](https://redirect.github.com/zarf-dev/zarf/compare/v0.40.0...v0.40.1)

#### What's Changed

- chore(deps): bump actions/create-github-app-token from 1.10.3 to
1.10.4 by [@&#8203;dependabot](https://redirect.github.com/dependabot)
in
[zarf-dev/zarf#2968
- fix: imported helm overrides by
[@&#8203;rjferguson21](https://redirect.github.com/rjferguson21) in
[zarf-dev/zarf#2967
- chore: only show config file if there is one by
[@&#8203;catsby](https://redirect.github.com/catsby) in
[zarf-dev/zarf#2985
- refactor: trim named returns in pkg
[#&#8203;2950](https://redirect.github.com/zarf-dev/zarf/issues/2950) by
[@&#8203;mkcp](https://redirect.github.com/mkcp) in
[zarf-dev/zarf#2979
- chore: finish removing named returns outside of package and extensions
[#&#8203;2950](https://redirect.github.com/zarf-dev/zarf/issues/2950) by
[@&#8203;mkcp](https://redirect.github.com/mkcp) in
[zarf-dev/zarf#2987
- chore: ensure we return zeroed value when returning errors by
[@&#8203;mkcp](https://redirect.github.com/mkcp) in
[zarf-dev/zarf#2988
- chore(deps): bump actions/create-github-app-token from 1.10.4 to
1.11.0 by [@&#8203;dependabot](https://redirect.github.com/dependabot)
in
[zarf-dev/zarf#2991
- refactor: break --insecure into separate flags by
[@&#8203;joonas](https://redirect.github.com/joonas) in
[zarf-dev/zarf#2936
- ci: stop codeql on merge queue by
[@&#8203;AustinAbro321](https://redirect.github.com/AustinAbro321) in
[zarf-dev/zarf#2934
- fix: add shasum flag and test for https pull by
[@&#8203;AustinAbro321](https://redirect.github.com/AustinAbro321) in
[zarf-dev/zarf#2998
- chore(deps): bump github/codeql-action from 3.26.6 to 3.26.7 by
[@&#8203;dependabot](https://redirect.github.com/dependabot) in
[zarf-dev/zarf#2997
- refactor: pull command by
[@&#8203;phillebaba](https://redirect.github.com/phillebaba) in
[zarf-dev/zarf#2989
- docs: update dos-games refs by
[@&#8203;jasonwashburn](https://redirect.github.com/jasonwashburn) in
[zarf-dev/zarf#3004
- refactor: lint by
[@&#8203;phillebaba](https://redirect.github.com/phillebaba) in
[zarf-dev/zarf#3000
- refactor: mirror-resources by
[@&#8203;phillebaba](https://redirect.github.com/phillebaba) in
[zarf-dev/zarf#2975
- fix: gittributes to ignore image file endings by
[@&#8203;phillebaba](https://redirect.github.com/phillebaba) in
[zarf-dev/zarf#3012

#### New Contributors

- [@&#8203;rjferguson21](https://redirect.github.com/rjferguson21) made
their first contribution in
[zarf-dev/zarf#2967
- [@&#8203;catsby](https://redirect.github.com/catsby) made their first
contribution in
[zarf-dev/zarf#2985
- [@&#8203;mkcp](https://redirect.github.com/mkcp) made their first
contribution in
[zarf-dev/zarf#2979
- [@&#8203;joonas](https://redirect.github.com/joonas) made their first
contribution in
[zarf-dev/zarf#2936

**Full Changelog**:
zarf-dev/zarf@v0.39.0...v0.40.1

###
[`v0.40.0`](https://redirect.github.com/zarf-dev/zarf/compare/v0.39.0...v0.40.0)

[Compare
Source](https://redirect.github.com/zarf-dev/zarf/compare/v0.39.0...v0.40.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/defenseunicorns/uds-core).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44MC4wIiwidXBkYXRlZEluVmVyIjoiMzguODAuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return nil values when returning an error
2 participants