You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/12/05 15:04:29 UTC
[GitHub] [beam] damccorm commented on a diff in pull request #24525: Run go fmt over full go directory with go 1.19
damccorm commented on code in PR #24525:
URL: https://github.com/apache/beam/pull/24525#discussion_r1039696386
##########
sdks/go/pkg/beam/io/xlang/bigqueryio/bigquery.go:
##########
@@ -37,28 +37,29 @@
//
// Current supported SDKs, including expansion service modules and reference
// documentation:
-// * Java
-// - Vendored Module: beam-sdks-java-extensions-schemaio-expansion-service
-// - Run via Gradle: ./gradlew :sdks:java:extensions:schemaio-expansion-service:runExpansionService
-// - Reference Class: org.apache.beam.sdk.io.gcp.bigquery.BigQuerySchemaIOProvider and
-// org.apache.beam.sdk.io.gcp.bigquery.BigQueryIO
+// - Java
+// - Vendored Module: beam-sdks-java-extensions-schemaio-expansion-service
+// - Run via Gradle: ./gradlew :sdks:java:extensions:schemaio-expansion-service:runExpansionService
+// - Reference Class: org.apache.beam.sdk.io.gcp.bigquery.BigQuerySchemaIOProvider and
Review Comment:
I updated this so that it renders as:
```
Java:
* bullets
* ...
* ...
```
instead of the way it currently renders:
```
* Java
* bullets
* ...
* ...
```
Current rendering here - https://pkg.go.dev/github.com/apache/beam/sdks/v2@v2.43.0/go/pkg/beam/io/xlang/bigqueryio#hdr-Setup
##########
sdks/go/pkg/beam/io/xlang/jdbcio/jdbc.go:
##########
@@ -35,10 +35,10 @@
// Current supported SDKs, including expansion service modules and reference
// documentation:
// * Java
-// - Vendored Module: beam-sdks-java-extensions-schemaio-expansion-service
-// - Run via Gradle: ./gradlew :sdks:java:extensions:schemaio-expansion-service:build
-// java -jar <location_of_jar_file_generated_from_above> <port>
-// - Reference Class: org.apache.beam.sdk.io.jdbc.JdbcIO
+// - Vendored Module: beam-sdks-java-extensions-schemaio-expansion-service
+// - Run via Gradle: ./gradlew :sdks:java:extensions:schemaio-expansion-service:build
+// java -jar <location_of_jar_file_generated_from_above> <port>
+// - Reference Class: org.apache.beam.sdk.io.jdbc.JdbcIO
Review Comment:
Same as bigquery/debezium
##########
sdks/go/pkg/beam/io/xlang/debeziumio/debezium.go:
##########
@@ -35,10 +35,10 @@
//
// Current supported SDKs including expansion service modules
// * Java
-// - Vendored Module: beam-sdks-java-io-debezium-expansion-service
-// - Run via Gradle: ./gradlew :sdks:java:io:debezium:expansion-service:shadowJar
-// java -jar <path-to-debezium-jar> <port>
-// - Reference Class: org.apache.beam.io.debezium.DebeziumIO
+// - Vendored Module: beam-sdks-java-io-debezium-expansion-service
+// - Run via Gradle: ./gradlew :sdks:java:io:debezium:expansion-service:shadowJar
+// java -jar <path-to-debezium-jar> <port>
+// - Reference Class: org.apache.beam.io.debezium.DebeziumIO
Review Comment:
I updated this for the same reason as the previous (bigquery one)
##########
sdks/go/pkg/beam/runners/universal/runnerlib/compile.go:
##########
@@ -55,10 +55,10 @@ func BuildTempWorkerBinary(ctx context.Context) (string, error) {
// BuildWorkerBinary creates a local worker binary for linux/amd64. It finds the filename
// by examining the call stack. We want the user entry (*), for example:
//
-// /Users/herohde/go/src/github.com/apache/beam/sdks/go/pkg/beam/runners/beamexec/main.go (skip: 2)
-// * /Users/herohde/go/src/github.com/apache/beam/sdks/go/examples/wordcount/wordcount.go (skip: 3)
-// /usr/local/go/src/runtime/proc.go (skip: 4) // not always present
-// /usr/local/go/src/runtime/asm_amd64.s (skip: 4 or 5)
+// /Users/herohde/go/src/github.com/apache/beam/sdks/go/pkg/beam/runners/beamexec/main.go (skip: 2)
+// * /Users/herohde/go/src/github.com/apache/beam/sdks/go/examples/wordcount/wordcount.go (skip: 3)
+// /usr/local/go/src/runtime/proc.go (skip: 4) // not always present
+// /usr/local/go/src/runtime/asm_amd64.s (skip: 4 or 5)
Review Comment:
This was very broken before - https://pkg.go.dev/github.com/apache/beam/sdks/v2@v2.43.0/go/pkg/beam/runners/universal/runnerlib#BuildWorkerBinary
Now it renders as:
<img width="1177" alt="image" src="https://user-images.githubusercontent.com/42773683/205669881-e36fa8ad-d908-4ba4-b9c8-63f7b3dd7ec1.png">
##########
sdks/go/pkg/beam/core/runtime/exec/fn_test.go:
##########
@@ -708,11 +708,11 @@ func (c *callerInt) Call1x1(a0 interface{}) interface{} {
// * Implicit or Explicit method Receiver
// * Pre-wrapped values and pre-allocated slices.
// * Invocations along the following ways
-// * Indirect via extracting from a reflect.Value.Interface()
-// * Reflect Package (reflect.Value.Call())
-// * Beam's reflectx.Func, and reflectx.FuncNxM interfaces
-// * Beam's default reflection based reflectx.Func shim
-// * A Type assertion specialized reflectx.Func shim
+// - Indirect via extracting from a reflect.Value.Interface()
+// - Reflect Package (reflect.Value.Call())
+// - Beam's reflectx.Func, and reflectx.FuncNxM interfaces
+// - Beam's default reflection based reflectx.Func shim
+// - A Type assertion specialized reflectx.Func shim
Review Comment:
Note that this is a change in behavior from 1.18 -> 1.19. 1.19 won't let you have subindented lists. Importantly, however, the following:
```
// * foo
// * bar
// * baz
```
has always rendered as:
```
* foo
* bar
* baz
```
in the godoc anyways (indents weren't maintained). I fixed a few of these cases in my second commit in this PR and called them out with comments. I left this one since I don't think the indents add anything.
##########
sdks/go/pkg/beam/io/xlang/kafkaio/kafka.go:
##########
@@ -36,9 +36,9 @@
// Current supported SDKs, including expansion service modules and reference
// documentation:
// * Java
-// - Vendored Module: beam-sdks-java-io-expansion-service
-// - Run via Gradle: ./gradlew :sdks:java:io:expansion-service:runExpansionService
-// - Reference Class: org.apache.beam.sdk.io.kafka.KafkaIO
+// - Vendored Module: beam-sdks-java-io-expansion-service
+// - Run via Gradle: ./gradlew :sdks:java:io:expansion-service:runExpansionService
+// - Reference Class: org.apache.beam.sdk.io.kafka.KafkaIO
Review Comment:
Same as bigquery/debezium/jdbc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org