Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Nick Kubala <nkubala@users.noreply.github.com>
  • Loading branch information
tejal29 and nkubala committed Dec 9, 2020
1 parent d48faf2 commit 72b5aac
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 15 deletions.
26 changes: 13 additions & 13 deletions pkg/skaffold/schema/latest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1179,13 +1179,13 @@ type JibArtifact struct {
BaseImage string `yaml:"fromImage,omitempty"`
}

// UnmarshalYAML provides a custom unmarshler to deal with
// UnmarshalYAML provides a custom unmarshaller to deal with
// https://github.com/GoogleContainerTools/skaffold/issues/4175
func (clusterDetails *ClusterDetails) UnmarshalYAML(value *yaml.Node) error {
// We do this as follows
// 1. We zero out the fields in the node that require custom processing
// 2. We unmarshall all the non special fields using the aliased type resource
// we use an alias type to avoid recursion caused by invoking this function infinetly
// 2. We unmarshal all the non special fields using the aliased type resource
// we use an alias type to avoid recursion caused by invoking this function infinitely
// 3. We deserialize the special fields as required.
type ClusterDetailsForUnmarshaling ClusterDetails

Expand All @@ -1207,13 +1207,13 @@ func (clusterDetails *ClusterDetails) UnmarshalYAML(value *yaml.Node) error {
return nil
}

// UnmarshalYAML provides a custom unmarshler to deal with
// UnmarshalYAML provides a custom unmarshaller to deal with
// https://github.com/GoogleContainerTools/skaffold/issues/4175
func (ka *KanikoArtifact) UnmarshalYAML(value *yaml.Node) error {
// We do this as follows
// 1. We zero out the fields in the node that require custom processing
// 2. We unmarshall all the non special fields using the aliased type resource
// we use an alias type to avoid recursion caused by invoking this function infinetly
// 2. We unmarshal all the non special fields using the aliased type resource
// we use an alias type to avoid recursion caused by invoking this function infinitely
// 3. We deserialize the special fields as required.
type KanikoArtifactForUnmarshaling KanikoArtifact

Expand All @@ -1235,15 +1235,15 @@ func (ka *KanikoArtifact) UnmarshalYAML(value *yaml.Node) error {
return nil
}

// MarshalYAML provides a custom marshler to deal with
// MarshalYAML provides a custom marshaller to deal with
// https://github.com/GoogleContainerTools/skaffold/issues/4175
func (clusterDetails *ClusterDetails) MarshalYAML() (interface{}, error) {
// We do this as follows
// 1. We zero out the fields in the node that require custom processing
// 2. We marshall all the non special fields using the aliased type resource
// we use an alias type to avoid recursion caused by invoking this function infinetly
// we use an alias type to avoid recursion caused by invoking this function infinitely
// 3. We unmarshal to a map
// 4. We marshall the special fields to json and unmarhsal to a map
// 4. We marshal the special fields to json and unmarshal to a map
// * This leverages the json struct annotations to marshal as expected
// 5. We combine the two maps and return
type ClusterDetailsForUnmarshaling ClusterDetails
Expand Down Expand Up @@ -1295,15 +1295,15 @@ func (clusterDetails *ClusterDetails) MarshalYAML() (interface{}, error) {
return m, err
}

// MarshalYAML provides a custom marshler to deal with
// MarshalYAML provides a custom marshaller to deal with
// https://github.com/GoogleContainerTools/skaffold/issues/4175
func (ka *KanikoArtifact) MarshalYAML() (interface{}, error) {
// We do this as follows
// 1. We zero out the fields in the node that require custom processing
// 2. We marshall all the non special fields using the aliased type resource
// we use an alias type to avoid recursion caused by invoking this function infinetly
// 2. We marshal all the non special fields using the aliased type resource
// we use an alias type to avoid recursion caused by invoking this function infinitely
// 3. We unmarshal to a map
// 4. We marshall the special fields to json and unmarhsal to a map
// 4. We marshal the special fields to json and unmarshal to a map
// * This leverages the json struct annotations to marshal as expected
// 5. We combine the two maps and return
type KanikoArtifactForUnmarshaling KanikoArtifact
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/schema/util/yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func UnmarshalClusterVolumes(value *yaml.Node) (volumes []v1.Volume, remaining [
}

// UnmarshalKanikoArtifact provides a helper function to
// for a custom unmarshler to deal with
// for a custom unmarshaller to deal with
// https://github.com/GoogleContainerTools/skaffold/issues/4175
func UnmarshalKanikoArtifact(value *yaml.Node) (mounts []v1.VolumeMount, remaining []byte, result error) {
kaMap := make(map[string]interface{})
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/schema/versions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ func TestMarshalConfig(t *testing.T) {
// Unmarshal the YAML and make sure it equals the original.
// We can't compare the strings because the YAML serializer isn't deterministic.
// TestParseConfigAndUpgrade verifies that YAML -> Go works correctly.
// This test veries Go -> YAML -> Go returns the original structure. Since we know
// This test verifies Go -> YAML -> Go returns the original structure. Since we know
// YAML -> Go is working this ensures Go -> YAML is correct.
recovered := &latest.SkaffoldConfig{}

Expand Down

0 comments on commit 72b5aac

Please sign in to comment.