Inject comments from .thrift into generated source

[jamesbroadhead]

Often, thrift definitions have useful comments, which are not immediately visible if navigating from code to generated source.

It could be helpful if scrooge generated javadoc-style comments from comments in thrift definitions and injected them into the generated source to help in cases like this.

[mosesn]

@jamesbroadhead I think we do, actually. See the gold file output. This is true across all of the kinds of code we generate, as far as I know.

[kevinoliver]

@mosesn My guess is that there are places where we miss injecting them.

@jamesbroadhead It'd be great if you could enumerate those places where it would help.

[kuroneko25]

We are seeing a similar problem where comments in thrift definitions are not included in the generated java source code.

We are using the scrooge maven plugin:

          <groupId>com.twitter</groupId>
          <artifactId>scrooge-maven-plugin</artifactId>
          <version>3.14.1</version>
          <configuration>
            <thriftSourceRoot>${basedir}/../thrift</thriftSourceRoot>
            <language>java</language>
            <thriftOpts>
              <!-- add other Scrooge command line options using thriftOpts -->
              <thriftOpt>--finagle</thriftOpt>
            </thriftOpts>
            <!-- tell scrooge to not to build the extracted thrift files (defaults to true) -->
            <buildExtractedThrift>false</buildExtractedThrift>
          </configuration>

And our thrift definition looks something like this:

...
/**
 * javadoc comment
 */
struct MyStruct {
  /* javadoc comment */
  1: string data;
}

service MyService {
  /**
   * javadoc comment
   */
  MyStruct myMethod(
    1: MyStruct input)
}
...

but none of the javadoc is included in the generated files.

[mosesn]

@kuroneko25 I don't think we support javadoc-style (two stars at the beginning), I think we only support c-style (one star at the beginning). Can you try again like so:

...
/*
 * javadoc comment
 */
struct MyStruct {
  /* javadoc comment */
  1: string data;
}

service MyService {
  /*
   * javadoc comment
   */
  MyStruct myMethod(
    1: MyStruct input)
}
...

It's definitely weird that the one inside the struct doesn't work. We should look into that.

[kevinoliver]

Moses, I think its only Doc-comment style (/** */) that is converted.

But it looks like we don't do this for the generated Java code. Note that
GoldService.java
https://github.com/twitter/scrooge/blob/develop/scrooge-generator-tests/src/test/resources/gold_file_output_java/com/twitter/scrooge/test/gold/thriftjava/GoldService.java
does
not have the "Hello, I'm a comment" comment that the Scala one has
https://github.com/twitter/scrooge/blob/develop/scrooge-generator-tests/src/test/resources/gold_file_output_scala/com/twitter/scrooge/test/gold/thriftscala/GoldService.scala#L32-L36
.

A PR to add this for the Java generator would be gladly accepted.

On Mon, Jun 27, 2016 at 1:52 PM, Moses Nakamura notifications@github.com
wrote:

@kuroneko25 https://github.com/kuroneko25 I don't think we support
javadoc-style (two stars at the beginning), I think we only support c-style
(one star at the beginning). Can you try again like so:

...
/*

• javadoc comment
  /
  struct MyStruct {
  / javadoc comment */
  1: string data;
  }

service MyService {
/*

• javadoc comment
  */
  MyStruct myMethod(
  1: MyStruct input)
  }
  ...

It's definitely weird that the one inside the struct doesn't work. We
should look into that.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#218 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAVMXUHspOJNa3BQldNEdpNiJYtzl6S3ks5qQDfzgaJpZM4HUO2E
.

Kevin Oliver | Twitter, Inc | follow me: @kevino http://twitter.com/kevino

[mosesn]

Whoops, my bad! I think in mainstream thrift, they only do /* */, we should look into supporting that too.