Updates base on PR feedback

Azure · Jul 21, 2021 · 3fb22f1 · 3fb22f1
1 parent fe69aba
commit 3fb22f1
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 11 deletions.
diff --git a/hack/generator/pkg/astmodel/interface_injector.go b/hack/generator/pkg/astmodel/interface_injector.go
@@ -24,8 +24,8 @@ func NewInterfaceInjector() *InterfaceInjector {
 }
 
 // Inject modifies the passed type definition by injecting the passed function
-func (fi *InterfaceInjector) Inject(def TypeDefinition, implementation *InterfaceImplementation) (TypeDefinition, error) {
-	result, err := fi.visitor.VisitDefinition(def, implementation)
+func (i *InterfaceInjector) Inject(def TypeDefinition, implementation *InterfaceImplementation) (TypeDefinition, error) {
+	result, err := i.visitor.VisitDefinition(def, implementation)
 	if err != nil {
 		return TypeDefinition{}, err
 	}
@@ -34,15 +34,15 @@ func (fi *InterfaceInjector) Inject(def TypeDefinition, implementation *Interfac
 
 // injectFunctionIntoObject takes the function provided as a context and includes it on the
 // provided object type
-func (_ *InterfaceInjector) injectInterfaceIntoObject(
+func (i *InterfaceInjector) injectInterfaceIntoObject(
 	_ *TypeVisitor, ot *ObjectType, ctx interface{}) (Type, error) {
 	implementation := ctx.(*InterfaceImplementation)
 	return ot.WithInterface(implementation), nil
 }
 
 // injectFunctionIntoResource takes the function provided as a context and includes it on the
 // provided resource type
-func (_ *InterfaceInjector) injectInterfaceIntoResource(
+func (i *InterfaceInjector) injectInterfaceIntoResource(
 	_ *TypeVisitor, rt *ResourceType, ctx interface{}) (Type, error) {
 	fn := ctx.(*InterfaceImplementation)
 	return rt.WithInterface(fn), nil

diff --git a/hack/generator/pkg/codegen/pipeline/implement_convertible_interface.go b/hack/generator/pkg/codegen/pipeline/implement_convertible_interface.go
@@ -62,6 +62,10 @@ func ImplementConvertibleInterface(idFactory astmodel.IdentifierFactory) Stage {
 	return stage
 }
 
+// createConvertibleInterfaceImplementation creates the required implementation of conversion.Convertible, ready for
+// injection onto the resource. The ConvertTo() and ConvertFrom() methods chain the required conversion between resource
+// versions, but are dependent upon previously injected AssignPropertiesTo() and AssignPropertiesFrom() methods to
+// actually copy information across. See resource_conversion_function.go for more information.
 func createConvertibleInterfaceImplementation(
 	name astmodel.TypeName,
 	resource *astmodel.ResourceType,
@@ -78,4 +82,4 @@ func createConvertibleInterfaceImplementation(
 	}
 
 	return astmodel.NewInterfaceImplementation(astmodel.ConvertibleInterface, conversionFunctions...)
-}
+}
diff --git a/hack/generator/pkg/codegen/pipeline/implement_convertible_interface_test.go b/hack/generator/pkg/codegen/pipeline/implement_convertible_interface_test.go
@@ -15,6 +15,8 @@ import (
 	"github.com/Azure/azure-service-operator/hack/generator/pkg/test"
 )
 
+// TestInjectConvertibleInterface checks that the pipeline stage does what we expect when run in relative isolation,
+// with only a few expected (and closely reated) stages in operation
 func TestInjectConvertibleInterface(t *testing.T) {
 	g := NewGomegaWithT(t)
 

diff --git a/hack/generator/pkg/functions/resource_conversion_function.go b/hack/generator/pkg/functions/resource_conversion_function.go
@@ -20,26 +20,28 @@ import (
 //
 // Direct conversion from the hub type:
 //
-// func (r <receiver>) ConvertFrom(hub conversion.Hub) error {
+// func (<receiver> <receiverType>) Convert<From|To>(hub conversion.Hub) error {
 // 	source, ok := hub.(*<hub.Type>)
 // 	if !ok {
 // 		return fmt.Errorf("expected <hub.Type> but received %T instead", hub)
 // 	}
-// 	return person.AssignPropertiesFromPerson(source)
+// 	return <receiver>.AssignProperties<From|To><Type>(source)
 // }
 //
 // Indirect conversion, multiple steps via an intermediate instance
 //
 // func (r <receiver>) Convert<From|To>(hub conversion.Hub) error {
 // 	var source <vNext>
-// 	err := source.ConvertFrom(hub)
+// 	err := source.Convert<From|To>(hub)
 // 	if err != nil {
 // 		return errors.Wrap(err, "converting from hub to source")
 // 	}
-// 	err = person.AssignPropertiesFromPerson(&source)
+//
+// 	err = <receiver>.AssignProperties<From|To><Type>(&source)
 // 	if err != nil {
-// 		return errors.Wrap(err, "converting from source to person")
+// 		return errors.Wrap(err, "converting from source to <type>")
 // 	}
+//
 // 	return nil
 // }
 //
@@ -137,7 +139,7 @@ func (fn *ResourceConversionFunction) AsFunc(
 //     return errors.Errorf("expected <actualHubType> but received %T instead", <hubAsInterface>)
 // }
 //
-// return receiver.AssignProperties(To|From)<type>(<local>)
+// return <receiver>.AssignProperties(To|From)<type>(<local>)
 //
 func (fn *ResourceConversionFunction) directConversion(
 	receiverName string, generationContext *astmodel.CodeGenerationContext) []dst.Stmt {
@@ -175,10 +177,12 @@ func (fn *ResourceConversionFunction) directConversion(
 // if err != nil {
 //     return errors.Wrap(err, "converting from hub to source")
 // }
+//
 // err = <receiver>.AssignPropertiesFrom<type>(&source)
 // if err != nil {
 //     return errors.Wrap(err, "converting from source to <type>")
 // }
+//
 // return nil
 //
 func (fn *ResourceConversionFunction) indirectConversionFromHub(
@@ -233,10 +237,12 @@ func (fn *ResourceConversionFunction) indirectConversionFromHub(
 // if err != nil {
 //     return errors.Wrap(err, "converting to destination from <type>")
 // }
+//
 // err := destination.ConvertTo(<hub>)
 // if err != nil {
 //     return errors.Wrap(err, "converting from destination to hub")
 // }
+//
 // return nil
 //
 func (fn *ResourceConversionFunction) indirectConversionToHub(