henryken commented on a change in pull request #11564:
URL: https://github.com/apache/beam/pull/11564#discussion_r418443043
##########
File path: learning/katas/go/Core Transforms/Map/ParDo/pkg/task/task.go
##########
@@ -18,8 +18,9 @@ package task
import "github.com/apache/beam/sdks/go/pkg/beam"
func ApplyTransform(s beam.Scope, input beam.PCollection) beam.PCollection {
- processFn := func(element int) int {
- return element * 10
- }
- return beam.ParDo(s, processFn, input)
+ return beam.ParDo(s, multiplyBy10Fn, input)
}
+
+func multiplyBy10Fn(element int) int {
+ return element * 10
Review comment:
The "* 10" is not covered by the answer placeholder. Is it intentional?
##########
File path: learning/katas/go/Core Transforms/Map/lesson-info.yaml
##########
@@ -20,5 +20,4 @@
content:
- ParDo
- ParDo OneToMany
-- MapElements
-- FlatMapElements
+- ParDo struct
Review comment:
Can maybe slightly rename to "ParDo Struct"?
##########
File path: learning/katas/go/Core Transforms/Map/ParDo struct/pkg/task/task.go
##########
@@ -18,10 +18,7 @@ package task
import "github.com/apache/beam/sdks/go/pkg/beam"
func ApplyTransform(s beam.Scope, input beam.PCollection) beam.PCollection {
- processFn := &multiplyByFn{
- Factor: 5,
- }
- return beam.ParDo(s, processFn, input)
+ return beam.ParDo(s, &multiplyByFn{Factor: 5}, input)
Review comment:
How about having the whole "&multiplyByFn{...}" be part of the answer
placeholder?
##########
File path: learning/katas/go/Core Transforms/Map/ParDo struct/task.md
##########
@@ -16,10 +16,10 @@
specific language governing permissions and limitations
under the License.
-->
-# Mapping Elements using structs
+# Using a struct as a DoFn
Review comment:
Can we have the ParDo mentioned, e.g. "ParDo - using a struct as a DoFn"?
##########
File path: learning/katas/go/Core Transforms/Map/ParDo
OneToMany/pkg/task/task.go
##########
@@ -21,10 +21,10 @@ import (
)
func ApplyTransform(s beam.Scope, input beam.PCollection) beam.PCollection {
- return beam.ParDo(s, processFn, input)
+ return beam.ParDo(s, tokenizeFn, input)
}
-func processFn(input string, emit func(out string)) {
+func tokenizeFn(input string, emit func(out string)) {
Review comment:
The answer placeholder(s) seems weird here. Maybe it requires a fix?
Or alternatively, we can use a similar answer placeholder as in the "ParDo"
task, i.e. having just empty "fund ApplyTransforms"?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
