You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by fhueske <gi...@git.apache.org> on 2014/12/12 12:01:31 UTC

[GitHub] incubator-flink pull request: [FLINK-1168] Adds multi-char field d...

GitHub user fhueske opened a pull request:

    https://github.com/apache/incubator-flink/pull/264

    [FLINK-1168] Adds multi-char field delimiter support for CsvInputFormats

    This commit includes parts of Cbro's pull request and subsumes PR #247
    
    This closes #247

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/fhueske/incubator-flink multiCharDelim

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-flink/pull/264.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 #264
    
----
commit be532817e00fa03050530e5995a3675740eb070d
Author: Fabian Hueske <fh...@apache.org>
Date:   2014-10-20T13:18:20Z

    [FLINK-1168] Added support for multi-char delimiters.
    This commit includes parts of Cbro's pull request and subsumes PR #247
    
    This closes #247

----


---
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] incubator-flink pull request: [FLINK-1168] Adds multi-char field d...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/incubator-flink/pull/264#issuecomment-66978329
  
    The question is whether we want to keep the char-delimiter in the user-facing API. 
    I would remove it in the long run to have the consistent data types for record and field delimiters. 
    That's why I marked the char variant as deprecated and switched all occurrences to String as we shouldn't encourage use of deprecated methods.
    
    Alternatively, we could add a char variant for the record delimiter but I don't think this would give a benefit for users.
    
    Btw. if we keep the char versions the PR would still change about 40 files.


---
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] incubator-flink pull request: [FLINK-1168] Adds multi-char field d...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:

    https://github.com/apache/incubator-flink/pull/264#issuecomment-66972764
  
    I vote to retain the methods that take a char as an argument. That way we retain backwards compatibility and do not break all existing programs.
    
    The number of changed files for this patch will probably be more like 5 then, rather than 67.


---
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.
---