You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@streams.apache.org by steveblackmon <gi...@git.apache.org> on 2014/12/05 20:09:35 UTC

[GitHub] incubator-streams pull request: Streams 242

GitHub user steveblackmon opened a pull request:

    https://github.com/apache/incubator-streams/pull/155

    Streams 242

    confirmed this eliminates Jenkins test failures in streams-provider-datasift

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

    $ git pull https://github.com/apache/incubator-streams STREAMS-242

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

    https://github.com/apache/incubator-streams/pull/155.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 #155
    
----
commit b57903f1ab0abd1ab7555c70dd454266aee69b83
Author: sblackmon <sb...@w2odigital.com>
Date:   2014-12-04T19:23:02Z

    require UTF-8 encoding in more places

commit 06fd10ef796c5d308f9a2c2bbc2561a5316c8749
Author: sblackmon <sb...@w2odigital.com>
Date:   2014-12-04T21:56:29Z

    specify UTF-8 encoding in Scanner
    line break support

commit f7c7508f0b6aad327794e22771b208a5ec3dbca4
Author: sblackmon <sb...@w2odigital.com>
Date:   2014-12-05T18:32:35Z

    specify UTF-8 encoding in Scanner
    line break support

----


---
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-streams pull request: Streams 242

Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/155#discussion_r21408264
  
    --- Diff: pom.xml ---
    @@ -125,6 +127,7 @@
                     <configuration>
                         <source>${java.version}</source>
                         <target>${java.version}</target>
    +                    <encoding>UTF-8</encoding>
    --- End diff --
    
    agree with using a property, but actually think it should be ${project.build.sourceEncoding}



---
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-streams pull request: Streams 242

Posted by robdouglas <gi...@git.apache.org>.
Github user robdouglas commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/155#discussion_r21403786
  
    --- Diff: pom.xml ---
    @@ -169,6 +172,14 @@
                         </execution>
                     </executions>
                 </plugin>
    +            <plugin>
    +                <groupId>org.apache.maven.plugins</groupId>
    +                <artifactId>maven-resources-plugin</artifactId>
    +                <version>2.7</version>
    +                <configuration>
    +                    <encoding>UTF-8</encoding>
    --- End diff --
    
    Shouldn't this use  ${project.reporting.outputEncoding} ?


---
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-streams pull request: Streams 242

Posted by robdouglas <gi...@git.apache.org>.
Github user robdouglas commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/155#discussion_r21403865
  
    --- Diff: streams-contrib/streams-provider-datasift/src/test/java17/org/apache/streams/datasift/serializer/DatasiftEventClassifierTest.java ---
    @@ -50,7 +57,10 @@ public void testTwitterDetection() throws Exception {
     
         @Test
         public void testInstagramDetection() throws Exception {
    -        Scanner scanner = new Scanner(DatasiftActivitySerializerTest.class.getResourceAsStream("/instagram_datasift_json.txt"));
    +
    --- End diff --
    
    This pattern is repeated several times, would it make sense to abstract it out to a utility function where the user could pass in the resource name and get back a Scanner object? That way if we needed to change encodings again for some reason, it wouldn't be as painful


---
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-streams pull request: Streams 242

Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/155#discussion_r21408403
  
    --- Diff: streams-contrib/streams-provider-datasift/src/test/java17/org/apache/streams/datasift/serializer/DatasiftEventClassifierTest.java ---
    @@ -50,7 +57,10 @@ public void testTwitterDetection() throws Exception {
     
         @Test
         public void testInstagramDetection() throws Exception {
    -        Scanner scanner = new Scanner(DatasiftActivitySerializerTest.class.getResourceAsStream("/instagram_datasift_json.txt"));
    +
    --- End diff --
    
    agree, i'll factor it out to a new utility class in streams-runtime-local:test-jar where any module that needs a solid scanner can access it.


---
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-streams pull request: Streams 242

Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/155#discussion_r21408477
  
    --- Diff: pom.xml ---
    @@ -169,6 +172,14 @@
                         </execution>
                     </executions>
                 </plugin>
    +            <plugin>
    +                <groupId>org.apache.maven.plugins</groupId>
    +                <artifactId>maven-resources-plugin</artifactId>
    +                <version>2.7</version>
    +                <configuration>
    +                    <encoding>UTF-8</encoding>
    --- End diff --
    
    agree with using a property, but actually think the most reasonable project-wide default would be ${project.build.sourceEncoding}, since all resource files the build uses are in the source tree.


---
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-streams pull request: Streams 242

Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/155#discussion_r21409517
  
    --- Diff: streams-contrib/streams-provider-datasift/src/test/java17/org/apache/streams/datasift/serializer/DatasiftEventClassifierTest.java ---
    @@ -50,7 +57,10 @@ public void testTwitterDetection() throws Exception {
     
         @Test
         public void testInstagramDetection() throws Exception {
    -        Scanner scanner = new Scanner(DatasiftActivitySerializerTest.class.getResourceAsStream("/instagram_datasift_json.txt"));
    +
    --- End diff --
    
    decided to put this in streams-util:test-jar, which is smaller and more sensible as a test import for a test that just needs decent access to local files than runtime-local


---
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-streams pull request: Streams 242

Posted by robdouglas <gi...@git.apache.org>.
Github user robdouglas commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/155#discussion_r21403785
  
    --- Diff: pom.xml ---
    @@ -125,6 +127,7 @@
                     <configuration>
                         <source>${java.version}</source>
                         <target>${java.version}</target>
    +                    <encoding>UTF-8</encoding>
    --- End diff --
    
    Shouldn't this use  ${project.reporting.outputEncoding} ?


---
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-streams pull request: Streams 242

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-streams/pull/155


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