chore: refactor actions code
#2276
Conversation
Racer159
requested review from
jeff-mccoy,
Noxsios,
lucasrod16 and
Madeline-UX
as code owners
✅ Deploy Preview for zarf-docs canceled.
|
Racer159
added
needs-adr
needs-docs
| func validateActionset(actions types.ZarfComponentActionSet) (bool, error) { | ||
| func validateActionset(actionSet actions.ActionSet) (bool, error) { | ||
| containsVariables := false | ||
|
|
There was a problem hiding this comment.
imo this could now be changed to a receiver on actions.ActionSet. Reason this was a function that took in an ActionSet was because it was from types and you cannot put receivers on types from other packages.
There was a problem hiding this comment.
if we move actions to the common pkg, this code will have to be brought over as well
There was a problem hiding this comment.
I decided to only pull actions.Validate() over - this logic is partly Zarf specific since it is looking for variables which only matters because Zarf has actions that don't support them (the rest of the logic is just simply looping through actions in the actionset)
src/pkg/utils/io.go
Outdated
| // MakeTempDir creates a temp directory with the zarf- prefix. | ||
| func MakeTempDir(basePath string) (string, error) { | ||
| if basePath != "" { | ||
| if err := CreateDirectory(basePath, 0700); err != nil { | ||
| if err := helpers.CreateDirectory(basePath, 0700); err != nil { | ||
| return "", err | ||
| } | ||
| } |
There was a problem hiding this comment.
MakeTempDir can prob be brought over to helpers (unless the internal debug is what made you keep it here)
There was a problem hiding this comment.
It was also the temp path prefix - IMO it is relatively Zarf specific - os.MkdirTemp is there for folks that want an OOB experience.
| Errorf(err error, format string, a ...any) | ||
| Successf(format string, a ...any) | ||
| Updatef(format string, a ...any) | ||
| Write(p []byte) (n int, err error) |
There was a problem hiding this comment.
Are you thinking this is a spinner or progressbar so someone may want to write progress? Would you ever be able to measure how much of an action is really done though?
There was a problem hiding this comment.
Spinner, the write method on spinner works differently from progressbar in that it takes a stdOut/stdErr stream
## Description #2180 introduced a bug and this PR removes the cause of the bug. Actions conditionals are being added to Zarf in #2276 to allow these sort of checks to account for various use cases in a more clean way. Also reopened #1824 ## Related Issue Relates to #2273 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [x] Test, docs, adr added or updated as needed - [x] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed
Racer159
changed the title
actions code
|
After discussing with @Racer159, this PR will not end up being merged, so closing this PR. The |
Description
This PR introduces conditionals to Zarf actions to help them function in a cross-platform manner.
Related Issue
Relates to #2253
Relates to #2273
Type of change
Checklist before merging