Skip to content

Commit

Permalink
fix: address some fixmes before merge
Browse files Browse the repository at this point in the history
Signed-off-by: Kit Patella <[email protected]>
  • Loading branch information
mkcp committed Sep 25, 2024
1 parent 3b274f7 commit a9d6266
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/cmd/destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var destroyCmd = &cobra.Command{
Aliases: []string{"d"},
Short: lang.CmdDestroyShort,
Long: lang.CmdDestroyLong,
// FIXME(mkcp): This function deeply needs a refactor and un-nesting.
// TODO(mkcp): refactor and de-nest this function.
RunE: func(cmd *cobra.Command, _ []string) error {
ctx := cmd.Context()
timeoutCtx, cancel := context.WithTimeout(cmd.Context(), cluster.DefaultTimeout)
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ func findInitPackage(ctx context.Context, initPackageName string) (string, error
return "", err
}
// Verify that we can write to the path
// FIXME(mkcp): Decompose this into a helper function
if helpers.InvalidPath(absCachePath) {
// Create the directory if the path is invalid
if err := helpers.CreateDirectory(absCachePath, helpers.ReadExecuteAllWriteUser); err != nil {
err = helpers.CreateDirectory(absCachePath, helpers.ReadExecuteAllWriteUser)
if err != nil {
return "", fmt.Errorf("unable to create the cache directory %s: %w", absCachePath, err)
}
}
Expand Down Expand Up @@ -138,7 +138,7 @@ func downloadInitPackage(ctx context.Context, cacheDirectory string) (string, er
message.Note(lang.CmdInitPullNote)

// Prompt the user if --confirm not specified
// FIXME(mkcp): This condition can never be met
// REVIEW(mkcp): It looks like this condition can't be met - maybe --confirm was removed at some point?
if !confirmDownload {
prompt := &survey.Confirm{
Message: lang.CmdInitPullConfirm,
Expand Down
3 changes: 1 addition & 2 deletions src/pkg/utils/cosign.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ func GetCosignArtifacts(image string) ([]string, error) {
return nil, err
}

// FIXME(mkcp): Re-review how this function is used and whether a signed entity err-miss is something we should pass
// up to the caller. Needs a comment if nothing else.
// Return empty if we don't have a signature on the image
var remoteOpts []ociremote.Option
simg, _ := ociremote.SignedEntity(ref, remoteOpts...)
if simg == nil {
Expand Down
9 changes: 6 additions & 3 deletions src/test/external/ext_in_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,12 @@ func (suite *ExtInClusterTestSuite) Test_1_Deploy() {
suite.NoError(err, "unable to teardown zarf")

// FIXME(mkcp): This code always fails to clean up the tmpdir. We're passing it a directory with contents and we
// need os.RemoveAll(temp) instead. However, we're also getting what looks to be a bug with concurrent writers
// causing a subtest to panic when we call RemoveAll(). Fixing this would require removing the shared state.
_ = os.Remove(temp) //nolint:errcheck
// need os.RemoveAll(temp) instead. However, we're also getting what looks to be a bug with concurrent
// writers causing a subtest to panic when we call RemoveAll(). Fixing this would require removing the
// shared state.
// REVIEW(mkcp): Thinking we delete this and
err = os.RemoveAll(temp) //nolint:errcheck
suite.NoError(err, "failed to clean up tempdir")
}

func TestExtInClusterTestSuite(t *testing.T) {
Expand Down

0 comments on commit a9d6266

Please sign in to comment.