damccorm commented on code in PR #23057:
URL: https://github.com/apache/beam/pull/23057#discussion_r964297924
##########
sdks/go/examples/wordcount/wordcount.go:
##########
@@ -93,39 +93,47 @@ var (
output = flag.String("output", "", "Output file (required).")
)
-// Concept #3: You can make your pipeline assembly code less verbose and by
+// Concept #3: You can make your pipeline assembly code less verbose by
// defining your DoFns statically out-of-line. A DoFn can be defined as a Go
// function and is conventionally suffixed "Fn". The argument and return types
-// dictate the pipeline shape when used in a ParDo: for example,
+// dictate the pipeline shape when used in a ParDo. For example,
//
-// formatFn: string x int -> string
+// formatFn: string x int -> string
Review Comment:
```suggestion
// func formatFn(w string, c int) string
```
Optional: if the current notation is causing confusion, we could consider
something like this (here and below)
##########
sdks/go/examples/wordcount/wordcount.go:
##########
@@ -137,7 +145,8 @@ var (
lineLen = beam.NewDistribution("extract", "lineLenDistro")
)
-// extractFn is a DoFn that emits the words in a given line and keeps a count
for small words.
+// extractFn is a structural DoFn that emits the words in a given line and
keeps
+// a count for small words.
Review Comment:
```suggestion
// a count for small words. Its ProcessElement function will be invoked on
each
// element in the input PCollection.
```
Optional: This might be helpful context if people aren't familiar with Beam.
Not sure if I complied w/ the 80 char limit
##########
sdks/go/examples/wordcount/wordcount.go:
##########
@@ -93,39 +93,47 @@ var (
output = flag.String("output", "", "Output file (required).")
)
-// Concept #3: You can make your pipeline assembly code less verbose and by
+// Concept #3: You can make your pipeline assembly code less verbose by
// defining your DoFns statically out-of-line. A DoFn can be defined as a Go
// function and is conventionally suffixed "Fn". The argument and return types
-// dictate the pipeline shape when used in a ParDo: for example,
+// dictate the pipeline shape when used in a ParDo. For example,
//
-// formatFn: string x int -> string
+// formatFn: string x int -> string
//
-// indicate that it operates on a PCollection of type KV<string,int>,
representing
-// key value pairs of strings and ints, and outputs a PCollection of type
string.
-// Beam typechecks the pipeline before running it.
+// indicates that the function operates on a PCollection of type
KV<string,int>,
+// representing key value pairs of strings and ints, and outputs a PCollection
+// of type string. Beam typechecks the pipeline before running it.
//
-// DoFns that potentially output zero or multiple elements can also be Go
functions,
-// but have a different signature. For example,
+// DoFns that potentially output zero or multiple elements can also be Go
+// functions, but have a different signature. For example,
//
-// extractFn : string x func(string) -> ()
+// extractFn : string x func(string) -> ()
//
-// uses an "emit" function argument instead of string return type to allow it
to
-// output any number of elements. It operates on a PCollection of type string
and
-// returns a PCollection of type string. Also, using named function transforms
allows
-// for easy reuse, modular testing, and an improved monitoring experience.
+// uses an "emit" function argument instead of a string return type to allow it
+// to output any number of elements. It operates on a PCollection of type
string
+// and returns a PCollection of type string. Also, using named function
+// transforms allows for easy reuse, modular testing, and an improved
monitoring
+// experience.
Review Comment:
Optional:
> Also, using named function transforms allows for easy reuse, modular
testing, and an improved monitoring experience.
Not introduced by you, but this feels like an awkward add-on that doesn't
really flow here IMO. Maybe it would be more appropriate above after: `A DoFn
can be defined as a Go function and is conventionally suffixed "Fn".`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
