Skip to content

fix(codegen-scala): correctly refer to descriptor object#452

Merged
raboof merged 2 commits intolightbend:mainfrom
raboof:correctly-reference-descriptor-object
Sep 22, 2021
Merged

fix(codegen-scala): correctly refer to descriptor object#452
raboof merged 2 commits intolightbend:mainfrom
raboof:correctly-reference-descriptor-object

Conversation

@raboof
Copy link
Copy Markdown
Contributor

@raboof raboof commented Sep 22, 2021

There is still a problem with the customer-registry sample
where Akka gRPC-generated code refers to AnnotationsProto
in a way that does not currently work, but that's a separate
issue

There is still a problem with the customer-registry sample
where Akka gRPC-generated code refers to AnnotationsProto
in a way that does not currently work, but that's a separate
issue
@patriknw
Copy link
Copy Markdown
Contributor

where Akka gRPC-generated code refers to AnnotationsProto

good that you confirm that, I was trying to understand that in another PR but didn't get anywhere

Copy link
Copy Markdown
Contributor

@patriknw patriknw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I guess the test must be adjusted

class ValueEntitySourceGeneratorSuite extends munit.FunSuite {
import com.akkaserverless.codegen.scalasdk.impl.ValueEntitySourceGenerator._

// TODO use package naming template parameter to generate Scala-style testData
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

|
| override final def serviceDescriptor: Descriptors.ServiceDescriptor =
| ${view.fqn.parent.name}Proto.javaDescriptor.findServiceByName("${view.fqn.protoName}")
| ${typeName(view.descriptorObject)}.javaDescriptor.findServiceByName("${view.fqn.protoName}")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no change in ViewServiceSourceGeneratorSuite?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, I suspect thats due to TestData not being entirely realistic still, not sure if it's worth digging into the details

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, it's defined as my_service.proto, which accidentally is the same as MyService

Comment on lines +42 to +44
override def fileDescriptorObject(descriptor: Descriptors.FileDescriptor): FullyQualifiedName =
// TODO move logic from generator classes to here
???
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is causing the build to fail

Copy link
Copy Markdown
Member

@octonato octonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but build is failing because of a ???

I'm looking forward to this as it's also hitting the Actions generations, as well as the AnnotationProto issue

@raboof raboof merged commit f7210ca into lightbend:main Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants