You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by StephanEwen <gi...@git.apache.org> on 2016/08/19 10:20:05 UTC
[GitHub] flink pull request #2390: [FLINK-4431] [core] Introduce a "VisibleForTesting...
GitHub user StephanEwen opened a pull request:
https://github.com/apache/flink/pull/2390
[FLINK-4431] [core] Introduce a "VisibleForTesting" annotation.
This annotations declares that a function, field, constructor, or entire type, is only visible for testing purposes.
This annotation is typically attached when for example a method should be {@code private} (because it is not intended to be called externally), but cannot be declared private, because some tests need to have access to it.
Guava offers a similar annotation, but I would like to not rely on Guava for this.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/StephanEwen/incubator-flink visible_testing_annotation
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/2390.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 #2390
----
commit 19fa1c6315750cbf001194300f4f7681e8be5e90
Author: Stephan Ewen <se...@apache.org>
Date: 2016-08-19T10:16:09Z
[FLINK-4431] [core] Introduce a "VisibleForTesting" annotation.
This annotation documents methods/fields that are not private because tests need them,
but should not be called by any non-testing code.
----
---
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 #2390: [FLINK-4431] [core] Introduce a "VisibleForTesting" annot...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2390
I would merge this and file for a followup issue to replace the Guava annotations and add a checkstyle rule.
---
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 #2390: [FLINK-4431] [core] Introduce a "VisibleForTesting" annot...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2390
+1, when adding this we should probably replace all uses of the `Guava` version and check that we don't use it using checkstyle rules. (If our goal is to one day remove our dependency on `Guava`.)
---
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 #2390: [FLINK-4431] [core] Introduce a "VisibleForTesting" annot...
Posted by ramkrish86 <gi...@git.apache.org>.
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/2390
+1. Very useful one. Also I saw some of the interfaces though they are supposed to be Public it is not marked with those types. Is it better to fix all of them ? Or add a test case that ensures that any new class/existing class if it misses annotation either Public or Private or what ever could be a new type, then we should fail the build. We have such a procedure in our project. Sorry if this is offtopic. Just thought of it so saying here.
---
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 #2390: [FLINK-4431] [core] Introduce a "VisibleForTesting" annot...
Posted by ramkrish86 <gi...@git.apache.org>.
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/2390
Thanks @StephanEwen . I can work on such Util. Will get back to 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] flink issue #2390: [FLINK-4431] [core] Introduce a "VisibleForTesting" annot...
Posted by ramkrish86 <gi...@git.apache.org>.
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/2390
@StephanEwen
Sorry for continuing in this thread. I did not meant to mean the StabilityAnnotations. I was only talking about other interfaces without annotations - Public/ Internal/PublicEvolving that flink is already using.
For eg - TimeStampAssigner and the other interfaces extending it. They don't have types annotated. I meant such interfaces.
---
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 #2390: [FLINK-4431] [core] Introduce a "VisibleForTesting" annot...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2390
@ramkrish86 Only some project are API projects and need annotations. Other internal projects have no stability annotations.
---
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 #2390: [FLINK-4431] [core] Introduce a "VisibleForTesting...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/2390
---
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 #2390: [FLINK-4431] [core] Introduce a "VisibleForTesting" annot...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the issue:
https://github.com/apache/flink/pull/2390
Makes sense +1
---
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 #2390: [FLINK-4431] [core] Introduce a "VisibleForTesting" annot...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2390
@ramkrish86 Within the public API projects, all classes should have such annotations. We currently have no automated way of checking that. Would not hurt to have that, I agree.
---
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.
---