Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-6127

Add MissingDeprecatedCheck to checkstyle

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.3.0
    • Component/s: Build System
    • Labels:
      None

      Description

      We should add the MissingDeprecatedCheck to our checkstyle rules to help avoiding deprecations without JavaDocs mentioning why the deprecation happened.

      http://checkstyle.sourceforge.net/apidocs/com/puppycrawl/tools/checkstyle/checks/annotation/MissingDeprecatedCheck.html

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user uce opened a pull request:

          https://github.com/apache/flink/pull/3572

          FLINK-6127 [checkstyle] Add MissingDeprecation check

          Adds the MissingDeprecation check to our set of checkstyle rules.

          Requires every `@Deprecated` annotation to have a `@deprecated` JavaDoc, forcing us to have both or none. This also applies for internal classes.

          Most of the changes were cosmetic (e.g. we had some comments but were not using the `@deprecated` JavaDoc).

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

          $ git pull https://github.com/uce/flink checkstyle_deprecation

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

          https://github.com/apache/flink/pull/3572.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 #3572


          commit 06d70219566a43049f07ff3159f556fe70dbcaec
          Author: Ufuk Celebi <uce@apache.org>
          Date: 2017-03-20T10:45:52Z

          FLINK-6127 [checkstyle] Add MissingDeprecation check

          commit a1f8b273dc8a662f38ebd92cf092f38f53008d35
          Author: Ufuk Celebi <uce@apache.org>
          Date: 2017-03-20T10:57:24Z

          FLINK-6127 [checkstyle] Fix missing @deprecated in flink-core

          commit 31377693c0e2ed95f0a6907059c65e21a6d78edf
          Author: Ufuk Celebi <uce@apache.org>
          Date: 2017-03-20T11:03:42Z

          FLINK-6127 [checkstyle] Fix missing @deprecated in flink-java

          commit de238a72a6920d6a34e6d36b5fdb62b23ab0c4ab
          Author: Ufuk Celebi <uce@apache.org>
          Date: 2017-03-20T11:20:38Z

          FLINK-6127 [checkstyle] Fix missing @deprecated in flink-runtime

          commit f909ab65f141ad4dcceb48fe616294adda43de53
          Author: Ufuk Celebi <uce@apache.org>
          Date: 2017-03-20T12:46:27Z

          FLINK-6127 [checkstyle] Fix missing @deprecated in flink-streaming-java

          commit b111cf665abcbb174d43a694f90570dc90c3da62
          Author: Ufuk Celebi <uce@apache.org>
          Date: 2017-03-20T12:48:27Z

          FLINK-6127 [checkstyle] Fix missing @deprecated in flink-statebackend-rocksdb

          commit 5532c8bbb02b716a5fd6b6e1d58f712c16ad5b55
          Author: Ufuk Celebi <uce@apache.org>
          Date: 2017-03-20T12:50:52Z

          FLINK-6127 [checkstyle] Fix missing @deprecated in flink-table

          commit 653a6f2c7848a0311c3ffbdfc3acc193101001c8
          Author: Ufuk Celebi <uce@apache.org>
          Date: 2017-03-20T12:55:51Z

          FLINK-6127 [checkstyle] Fix missing @deprecated in flink-connector-kafka-0.8

          commit 83e797d86f1bb8c07cf07b16517eb60bedf355d7
          Author: Ufuk Celebi <uce@apache.org>
          Date: 2017-03-20T12:57:55Z

          FLINK-6127 [checkstyle] Fix missing @deprecated in flink-yarn


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user uce opened a pull request: https://github.com/apache/flink/pull/3572 FLINK-6127 [checkstyle] Add MissingDeprecation check Adds the MissingDeprecation check to our set of checkstyle rules. Requires every `@Deprecated` annotation to have a `@deprecated` JavaDoc, forcing us to have both or none. This also applies for internal classes. Most of the changes were cosmetic (e.g. we had some comments but were not using the `@deprecated` JavaDoc). You can merge this pull request into a Git repository by running: $ git pull https://github.com/uce/flink checkstyle_deprecation Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3572.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 #3572 commit 06d70219566a43049f07ff3159f556fe70dbcaec Author: Ufuk Celebi <uce@apache.org> Date: 2017-03-20T10:45:52Z FLINK-6127 [checkstyle] Add MissingDeprecation check commit a1f8b273dc8a662f38ebd92cf092f38f53008d35 Author: Ufuk Celebi <uce@apache.org> Date: 2017-03-20T10:57:24Z FLINK-6127 [checkstyle] Fix missing @deprecated in flink-core commit 31377693c0e2ed95f0a6907059c65e21a6d78edf Author: Ufuk Celebi <uce@apache.org> Date: 2017-03-20T11:03:42Z FLINK-6127 [checkstyle] Fix missing @deprecated in flink-java commit de238a72a6920d6a34e6d36b5fdb62b23ab0c4ab Author: Ufuk Celebi <uce@apache.org> Date: 2017-03-20T11:20:38Z FLINK-6127 [checkstyle] Fix missing @deprecated in flink-runtime commit f909ab65f141ad4dcceb48fe616294adda43de53 Author: Ufuk Celebi <uce@apache.org> Date: 2017-03-20T12:46:27Z FLINK-6127 [checkstyle] Fix missing @deprecated in flink-streaming-java commit b111cf665abcbb174d43a694f90570dc90c3da62 Author: Ufuk Celebi <uce@apache.org> Date: 2017-03-20T12:48:27Z FLINK-6127 [checkstyle] Fix missing @deprecated in flink-statebackend-rocksdb commit 5532c8bbb02b716a5fd6b6e1d58f712c16ad5b55 Author: Ufuk Celebi <uce@apache.org> Date: 2017-03-20T12:50:52Z FLINK-6127 [checkstyle] Fix missing @deprecated in flink-table commit 653a6f2c7848a0311c3ffbdfc3acc193101001c8 Author: Ufuk Celebi <uce@apache.org> Date: 2017-03-20T12:55:51Z FLINK-6127 [checkstyle] Fix missing @deprecated in flink-connector-kafka-0.8 commit 83e797d86f1bb8c07cf07b16517eb60bedf355d7 Author: Ufuk Celebi <uce@apache.org> Date: 2017-03-20T12:57:55Z FLINK-6127 [checkstyle] Fix missing @deprecated in flink-yarn
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106906116

          — Diff: flink-libraries/flink-table/src/main/java/org/apache/flink/table/calcite/rules/FlinkAggregateJoinTransposeRule.java —
          @@ -78,22 +78,34 @@ public FlinkAggregateJoinTransposeRule(Class<? extends Aggregate> aggregateClass
          this.allowFunctions = allowFunctions;
          }

          • @Deprecated // to be removed before 2.0
            + /**
            + * @deprecated to be removed before 2.0
              • End diff –

          same as above, also multiple times in this file.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106906116 — Diff: flink-libraries/flink-table/src/main/java/org/apache/flink/table/calcite/rules/FlinkAggregateJoinTransposeRule.java — @@ -78,22 +78,34 @@ public FlinkAggregateJoinTransposeRule(Class<? extends Aggregate> aggregateClass this.allowFunctions = allowFunctions; } @Deprecated // to be removed before 2.0 + /** + * @deprecated to be removed before 2.0 End diff – same as above, also multiple times in this file.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106906265

          — Diff: flink-runtime/src/main/java/org/apache/flink/migration/runtime/checkpoint/TaskState.java —
          @@ -29,6 +29,9 @@
          import java.util.Objects;
          import java.util.Set;

          +/**
          + * @deprecated Internal class for savepoint backwards compatability. Don't use for other purposes.
          — End diff –

          typo: compatibility

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106906265 — Diff: flink-runtime/src/main/java/org/apache/flink/migration/runtime/checkpoint/TaskState.java — @@ -29,6 +29,9 @@ import java.util.Objects; import java.util.Set; +/** + * @deprecated Internal class for savepoint backwards compatability. Don't use for other purposes. — End diff – typo: compatibility
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106906205

          — Diff: flink-runtime/src/main/java/org/apache/flink/migration/runtime/checkpoint/SubtaskState.java —
          @@ -25,6 +25,9 @@

          import static org.apache.flink.util.Preconditions.checkNotNull;

          +/**
          + * @deprecated Internal class for savepoint backwards compatability. Don't use for other purposes.
          — End diff –

          typo: compatibility

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106906205 — Diff: flink-runtime/src/main/java/org/apache/flink/migration/runtime/checkpoint/SubtaskState.java — @@ -25,6 +25,9 @@ import static org.apache.flink.util.Preconditions.checkNotNull; +/** + * @deprecated Internal class for savepoint backwards compatability. Don't use for other purposes. — End diff – typo: compatibility
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106906033

          — Diff: flink-libraries/flink-table/src/main/java/org/apache/flink/table/calcite/rules/FlinkAggregateExpandDistinctAggregatesRule.java —
          @@ -114,15 +114,21 @@ public FlinkAggregateExpandDistinctAggregatesRule(
          this.useGroupingSets = useGroupingSets;
          }

          • @Deprecated // to be removed before 2.0
            + /**
            + * @deprecated to be removed before 2.0
              • End diff –

          Doesn't before imply that we remove it in earlier version than 2.0?

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106906033 — Diff: flink-libraries/flink-table/src/main/java/org/apache/flink/table/calcite/rules/FlinkAggregateExpandDistinctAggregatesRule.java — @@ -114,15 +114,21 @@ public FlinkAggregateExpandDistinctAggregatesRule( this.useGroupingSets = useGroupingSets; } @Deprecated // to be removed before 2.0 + /** + * @deprecated to be removed before 2.0 End diff – Doesn't before imply that we remove it in earlier version than 2.0?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106905405

          — Diff: flink-contrib/flink-statebackend-rocksdb/src/main/java/org/apache/flink/migration/contrib/streaming/state/RocksDBStateBackend.java —
          @@ -28,6 +28,9 @@

          import static java.util.Objects.requireNonNull;

          +/***
          — End diff –

          should be 2 stars

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106905405 — Diff: flink-contrib/flink-statebackend-rocksdb/src/main/java/org/apache/flink/migration/contrib/streaming/state/RocksDBStateBackend.java — @@ -28,6 +28,9 @@ import static java.util.Objects.requireNonNull; +/*** — End diff – should be 2 stars
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106905303

          — Diff: flink-contrib/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackend.java —
          @@ -1141,6 +1141,8 @@ public File getInstanceBasePath() {

          /**

          • For backwards compatibility, remove again later!
            + *
            + * @deprecated Internal method used for backwards compatability.
              • End diff –

          typo: compatibility

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106905303 — Diff: flink-contrib/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackend.java — @@ -1141,6 +1141,8 @@ public File getInstanceBasePath() { /** For backwards compatibility, remove again later! + * + * @deprecated Internal method used for backwards compatability. End diff – typo: compatibility
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106905341

          — Diff: flink-contrib/flink-statebackend-rocksdb/src/main/java/org/apache/flink/migration/contrib/streaming/state/RocksDBStateBackend.java —
          @@ -28,6 +28,9 @@

          import static java.util.Objects.requireNonNull;

          +/***
          + * @deprecated Internal class used for backwards compatability.
          — End diff –

          typo: compatibility

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106905341 — Diff: flink-contrib/flink-statebackend-rocksdb/src/main/java/org/apache/flink/migration/contrib/streaming/state/RocksDBStateBackend.java — @@ -28,6 +28,9 @@ import static java.util.Objects.requireNonNull; +/*** + * @deprecated Internal class used for backwards compatability. — End diff – typo: compatibility
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106906356

          — Diff: flink-runtime/src/main/java/org/apache/flink/migration/runtime/state/AbstractCloseableHandle.java —
          @@ -28,6 +28,8 @@

          • Offers to register a stream (or other closable object) that close calls are delegated to if
          • the handle is closed or was already closed.
            + *
            + * @deprecated Internal class for savepoint backwards compatability. Don't use for other purposes.
              • End diff –

          all right i give up, you go ahead and find all occurrences of "compatability"

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106906356 — Diff: flink-runtime/src/main/java/org/apache/flink/migration/runtime/state/AbstractCloseableHandle.java — @@ -28,6 +28,8 @@ Offers to register a stream (or other closable object) that close calls are delegated to if the handle is closed or was already closed. + * + * @deprecated Internal class for savepoint backwards compatability. Don't use for other purposes. End diff – all right i give up, you go ahead and find all occurrences of "compatability"
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106906046

          — Diff: flink-libraries/flink-table/src/main/java/org/apache/flink/table/calcite/rules/FlinkAggregateExpandDistinctAggregatesRule.java —
          @@ -114,15 +114,21 @@ public FlinkAggregateExpandDistinctAggregatesRule(
          this.useGroupingSets = useGroupingSets;
          }

          • @Deprecated // to be removed before 2.0
            + /**
            + * @deprecated to be removed before 2.0
            + */
            + @Deprecated
            public FlinkAggregateExpandDistinctAggregatesRule(
            Class<? extends LogicalAggregate> clazz,
            boolean useGroupingSets,
            RelFactories.JoinFactory joinFactory) { this(clazz, useGroupingSets, RelBuilder.proto(Contexts.of(joinFactory))); }
          • @Deprecated // to be removed before 2.0
            + /**
            + * @deprecated to be removed before 2.0
              • End diff –

          Same as above

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106906046 — Diff: flink-libraries/flink-table/src/main/java/org/apache/flink/table/calcite/rules/FlinkAggregateExpandDistinctAggregatesRule.java — @@ -114,15 +114,21 @@ public FlinkAggregateExpandDistinctAggregatesRule( this.useGroupingSets = useGroupingSets; } @Deprecated // to be removed before 2.0 + /** + * @deprecated to be removed before 2.0 + */ + @Deprecated public FlinkAggregateExpandDistinctAggregatesRule( Class<? extends LogicalAggregate> clazz, boolean useGroupingSets, RelFactories.JoinFactory joinFactory) { this(clazz, useGroupingSets, RelBuilder.proto(Contexts.of(joinFactory))); } @Deprecated // to be removed before 2.0 + /** + * @deprecated to be removed before 2.0 End diff – Same as above
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106906232

          — Diff: flink-runtime/src/main/java/org/apache/flink/migration/runtime/checkpoint/KeyGroupState.java —
          @@ -30,6 +30,8 @@
          *

          • The key group state handle is kept in serialized form because it can contain user code classes
          • which might not be available on the JobManager.
            + *
            + * @deprecated Internal class for savepoint backwards compatability. Don't use for other purposes.
              • End diff –

          typo: compatibility

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106906232 — Diff: flink-runtime/src/main/java/org/apache/flink/migration/runtime/checkpoint/KeyGroupState.java — @@ -30,6 +30,8 @@ * The key group state handle is kept in serialized form because it can contain user code classes which might not be available on the JobManager. + * + * @deprecated Internal class for savepoint backwards compatability. Don't use for other purposes. End diff – typo: compatibility
          Hide
          aljoscha Aljoscha Krettek added a comment -

          +1 This seems very helpful.

          Show
          aljoscha Aljoscha Krettek added a comment - +1 This seems very helpful.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user uce commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106942215

          — Diff: flink-libraries/flink-table/src/main/java/org/apache/flink/table/calcite/rules/FlinkAggregateExpandDistinctAggregatesRule.java —
          @@ -114,15 +114,21 @@ public FlinkAggregateExpandDistinctAggregatesRule(
          this.useGroupingSets = useGroupingSets;
          }

          • @Deprecated // to be removed before 2.0
            + /**
            + * @deprecated to be removed before 2.0
              • End diff –

          I don't know, this is independent imo as it's just copying the previous comment

          Show
          githubbot ASF GitHub Bot added a comment - Github user uce commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106942215 — Diff: flink-libraries/flink-table/src/main/java/org/apache/flink/table/calcite/rules/FlinkAggregateExpandDistinctAggregatesRule.java — @@ -114,15 +114,21 @@ public FlinkAggregateExpandDistinctAggregatesRule( this.useGroupingSets = useGroupingSets; } @Deprecated // to be removed before 2.0 + /** + * @deprecated to be removed before 2.0 End diff – I don't know, this is independent imo as it's just copying the previous comment
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user uce commented on the issue:

          https://github.com/apache/flink/pull/3572

          Thanks for catching the unfortunate typo. I've fixed all of those. I left the confusing deprecation comment as is in the table API.

          I think this is good to merge then.

          Show
          githubbot ASF GitHub Bot added a comment - Github user uce commented on the issue: https://github.com/apache/flink/pull/3572 Thanks for catching the unfortunate typo. I've fixed all of those. I left the confusing deprecation comment as is in the table API. I think this is good to merge then.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fhueske commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3572#discussion_r106967721

          — Diff: flink-libraries/flink-table/src/main/java/org/apache/flink/table/calcite/rules/FlinkAggregateExpandDistinctAggregatesRule.java —
          @@ -114,15 +114,21 @@ public FlinkAggregateExpandDistinctAggregatesRule(
          this.useGroupingSets = useGroupingSets;
          }

          • @Deprecated // to be removed before 2.0
            + /**
            + * @deprecated to be removed before 2.0
              • End diff –

          These are actually deprecation annotations by Calcite. We copied the files (see comment on top) because we had to fix a bug. The files will be removed once the bugs are fixed with a newer Calcite release.

          Show
          githubbot ASF GitHub Bot added a comment - Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/3572#discussion_r106967721 — Diff: flink-libraries/flink-table/src/main/java/org/apache/flink/table/calcite/rules/FlinkAggregateExpandDistinctAggregatesRule.java — @@ -114,15 +114,21 @@ public FlinkAggregateExpandDistinctAggregatesRule( this.useGroupingSets = useGroupingSets; } @Deprecated // to be removed before 2.0 + /** + * @deprecated to be removed before 2.0 End diff – These are actually deprecation annotations by Calcite. We copied the files (see comment on top) because we had to fix a bug. The files will be removed once the bugs are fixed with a newer Calcite release.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

          https://github.com/apache/flink/pull/3572

          merging.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on the issue: https://github.com/apache/flink/pull/3572 merging.
          Hide
          Zentol Chesnay Schepler added a comment -

          1.3: 68605d05107e2c70c12178b1db6a9e49641dbfe4

          Show
          Zentol Chesnay Schepler added a comment - 1.3: 68605d05107e2c70c12178b1db6a9e49641dbfe4
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3572

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3572

            People

            • Assignee:
              uce Ufuk Celebi
              Reporter:
              uce Ufuk Celebi
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development