You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "huwh (via GitHub)" <gi...@apache.org> on 2023/04/27 02:33:03 UTC

[GitHub] [flink] huwh commented on a diff in pull request #22438: [FLINK-31873] Add method setMaxParallelism to DataStreamSink.

huwh commented on code in PR #22438:
URL: https://github.com/apache/flink/pull/22438#discussion_r1178564640


##########
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/graph/StreamingJobGraphGeneratorTest.java:
##########
@@ -283,6 +283,25 @@ public void testTransformationSetParallelism() {
         assertThat(vertices.get(2).isParallelismConfigured()).isTrue();
     }
 
+    @Test
+    public void testTransformationSetMaxParallelism() {

Review Comment:
   Junit 4 requires all test methods to be public. We are currently migrating to Junit5, which does not have this requirement. And then to keep visibility to a minimum, it's better to make test methods package-private. 
   There are 5 test methods that are public, others are package-private. I guess it may be that the visibility was not noticed when these test methods were introduced. 
   
   Anyway, it's better to keep the same style in test cases. Would you mind changing them to package-private in a separate hotfix commit in this PR?



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org