You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by liuyuzhong7 <gi...@git.apache.org> on 2017/03/07 11:19:25 UTC
[GitHub] flink pull request #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
GitHub user liuyuzhong7 opened a pull request:
https://github.com/apache/flink/pull/3485
FLINK-5976 [tests] Deduplicate Tokenizer in tests
There are some duplicate code like this in flink-test, I think refactor this will be better.
```
public final class Tokenizer implements FlatMapFunction<String, Tuple2<String, Integer>> {
@Override
public void flatMap(String value, Collector<Tuple2<String, Integer>> out) {
// normalize and split the line
String[] tokens = value.toLowerCase().split("\\W+");
// emit the pairs
for (String token : tokens) {
if (token.length() > 0) {
out.collect(new Tuple2<String, Integer>(token, 1));
}
}
}
}
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/liuyuzhong7/flink FLINK-5976
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3485.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3485
----
commit db4192110b1b610846f50527a5fc67450d3d1cca
Author: liuyuzhong7 <li...@gmail.com>
Date: 2017-03-07T11:15:40Z
FLINK-5976 [tests] Deduplicate Tokenizer in tests
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/3485
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on the issue:
https://github.com/apache/flink/pull/3485
@uce
1. This pull request has squash in one commit.
2. JIRA has set affects version to 1.2.0
Help me to review this code, Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on the issue:
https://github.com/apache/flink/pull/3485
@StephanEwen It's a good idea. We can remove many duplicate code in flink-tests in this way.
But I found it also duplicate in other modules. How to solve it in other modules.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3485
Looks good, thanks! Merging this...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on the issue:
https://github.com/apache/flink/pull/3485
OK, get it. So how about just do it now in flink-tests like this pull request?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on a diff in the pull request:
https://github.com/apache/flink/pull/3485#discussion_r104826690
--- Diff: flink-tests/src/test/java/org/apache/flink/test/util/Tokenizer.java ---
@@ -0,0 +1,17 @@
+package org.apache.flink.test.util;
--- End diff --
@zentol fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3485
Would be nice to follow up here and collect also functions like "identity mapper" or so in that package.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/3485#discussion_r104650335
--- Diff: flink-tests/src/test/java/org/apache/flink/test/util/Tokenizer.java ---
@@ -0,0 +1,17 @@
+package org.apache.flink.test.util;
--- End diff --
this file is missing the apache license.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3485
I don't think it is necessary to unify all example functions. These are just minimal non critical pieces defined by some tests. One we drop Java 7, most of them will anyways disappear and become Lambdas. So I would not overdo this right now...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3485
How about we create a package `org.apache.flink.test.testfunctions` where we collect all shared reusable function implementations?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3485: FLINK-5976 [tests] Deduplicate Tokenizer in tests
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on the issue:
https://github.com/apache/flink/pull/3485
@StephanEwen
FlatMapFunction "Tokenizer" has move to org.apache.flink.test.testfunctions.
Help me to review this. Thanks.
And collect other reusable functions to org.apache.flink.test.testfunctions later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---