Solr
  1. Solr
  2. SOLR-489

Added @deprecation Javadoc comments

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3
    • Component/s: documentation
    • Labels:
      None

      Description

      In a number of files, @Deprecation annotations were added without accompanying @deprecation Javadoc comments to explain what to use now.

      1. deprecationDocumentation.patch
        14 kB
        Sean Timm
      2. SOLR-489.patch
        17 kB
        Lars Kotthoff
      3. SOLR-489.patch
        17 kB
        Lars Kotthoff
      4. SOLR-489.patch
        16 kB
        Lars Kotthoff
      5. SOLR-489.patch
        18 kB
        Lars Kotthoff

        Issue Links

          Activity

          Hide
          Sean Timm added a comment -

          Adds @deprecation comments to many of the files where they are missing.

          Show
          Sean Timm added a comment - Adds @deprecation comments to many of the files where they are missing.
          Hide
          Sean Timm added a comment -

          At Hoss' goading, here is my attempt at adding @deprecation tags where missing.

          The attached patch fixes adds comments where (I think) it is clear to me why something was deprecated and what to use now. Some files contain deprecations that I am less sure of--I didn't make any changes to those. They are primarily related to refactoring done in SOLR-135, SOLR-215, and SOLR-301.

          Additionally, I noticed that @Deprecated is commented out in two places in org/apache/solr/handler/XmlUpdateRequestHandler.java. I'm not sure if that is meant to be commented out or not.

          Here are the files which still have uncommented @Deprecation annotations.

          ./src/java/org/apache/solr/request/SolrQueryRequestBase.java
          ./src/java/org/apache/solr/request/SolrQueryRequest.java
          ./src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java
          ./src/java/org/apache/solr/handler/XmlUpdateRequestHandler.java
          ./src/java/org/apache/solr/tst/OldRequestHandler.java
          ./src/java/org/apache/solr/tst/TestRequestHandler.java
          ./src/java/org/apache/solr/util/TestHarness.java
          ./src/java/org/apache/solr/util/CommonParams.java
          ./src/java/org/apache/solr/core/Config.java
          ./src/java/org/apache/solr/core/SolrInfoRegistry.java
          ./src/java/org/apache/solr/core/SolrCore.java
          ./src/java/org/apache/solr/core/SolrConfig.java
          ./src/java/org/apache/solr/search/DocSet.java
          ./src/java/org/apache/solr/schema/IndexSchema.java
          ./src/java/org/apache/solr/schema/FieldType.java
          ./src/java/org/apache/solr/analysis/WordDelimiterFilter.java
          ./src/webapp/src/org/apache/solr/servlet/SolrUpdateServlet.java
          ./src/webapp/src/org/apache/solr/servlet/SolrServlet.java

          Show
          Sean Timm added a comment - At Hoss' goading, here is my attempt at adding @deprecation tags where missing. The attached patch fixes adds comments where (I think) it is clear to me why something was deprecated and what to use now. Some files contain deprecations that I am less sure of--I didn't make any changes to those. They are primarily related to refactoring done in SOLR-135 , SOLR-215 , and SOLR-301 . Additionally, I noticed that @Deprecated is commented out in two places in org/apache/solr/handler/XmlUpdateRequestHandler.java. I'm not sure if that is meant to be commented out or not. Here are the files which still have uncommented @Deprecation annotations. ./src/java/org/apache/solr/request/SolrQueryRequestBase.java ./src/java/org/apache/solr/request/SolrQueryRequest.java ./src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java ./src/java/org/apache/solr/handler/XmlUpdateRequestHandler.java ./src/java/org/apache/solr/tst/OldRequestHandler.java ./src/java/org/apache/solr/tst/TestRequestHandler.java ./src/java/org/apache/solr/util/TestHarness.java ./src/java/org/apache/solr/util/CommonParams.java ./src/java/org/apache/solr/core/Config.java ./src/java/org/apache/solr/core/SolrInfoRegistry.java ./src/java/org/apache/solr/core/SolrCore.java ./src/java/org/apache/solr/core/SolrConfig.java ./src/java/org/apache/solr/search/DocSet.java ./src/java/org/apache/solr/schema/IndexSchema.java ./src/java/org/apache/solr/schema/FieldType.java ./src/java/org/apache/solr/analysis/WordDelimiterFilter.java ./src/webapp/src/org/apache/solr/servlet/SolrUpdateServlet.java ./src/webapp/src/org/apache/solr/servlet/SolrServlet.java
          Hide
          Hoss Man added a comment -

          Thanks Sean ... reading the patch these all seem right to me so i've commited it.

          I think we should leave this issue open for now because of all the remaining instances you mentioned – ideally we'll get them all documented eventually.

          Show
          Hoss Man added a comment - Thanks Sean ... reading the patch these all seem right to me so i've commited it. I think we should leave this issue open for now because of all the remaining instances you mentioned – ideally we'll get them all documented eventually.
          Hide
          Hoss Man added a comment -

          Adding to the list for 1.3 ... we shouldn't release without @deprecation comments for every @Deprecation annotation.

          Show
          Hoss Man added a comment - Adding to the list for 1.3 ... we shouldn't release without @deprecation comments for every @Deprecation annotation.
          Hide
          Lars Kotthoff added a comment -

          Attaching patch which adds @deprecation comments to @Deprecation annotations for the following files:

          ./src/webapp/src/org/apache/solr/servlet/SolrServlet.java
          ./src/java/org/apache/solr/handler/XmlUpdateRequestHandler.java
          ./src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java
          ./src/java/org/apache/solr/tst/TestRequestHandler.java
          ./src/java/org/apache/solr/tst/OldRequestHandler.java
          ./src/java/org/apache/solr/analysis/WordDelimiterFilter.java
          ./src/java/org/apache/solr/schema/IndexSchema.java
          ./src/java/org/apache/solr/search/DocSet.java
          ./src/java/org/apache/solr/core/SolrCore.java
          ./src/java/org/apache/solr/core/SolrConfig.java
          ./src/java/org/apache/solr/core/Config.java
          ./src/java/org/apache/solr/request/SolrQueryRequest.java
          ./src/java/org/apache/solr/request/SolrQueryRequestBase.java
          ./src/java/org/apache/solr/util/CommonParams.java
          ./client/java/solrj/src/org/apache/solr/client/solrj/request/UpdateRequest.java

          That should be it, unless I've missed something. Somebody who knows the codebase better than me should have a look over it.

          Show
          Lars Kotthoff added a comment - Attaching patch which adds @deprecation comments to @Deprecation annotations for the following files: ./src/webapp/src/org/apache/solr/servlet/SolrServlet.java ./src/java/org/apache/solr/handler/XmlUpdateRequestHandler.java ./src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java ./src/java/org/apache/solr/tst/TestRequestHandler.java ./src/java/org/apache/solr/tst/OldRequestHandler.java ./src/java/org/apache/solr/analysis/WordDelimiterFilter.java ./src/java/org/apache/solr/schema/IndexSchema.java ./src/java/org/apache/solr/search/DocSet.java ./src/java/org/apache/solr/core/SolrCore.java ./src/java/org/apache/solr/core/SolrConfig.java ./src/java/org/apache/solr/core/Config.java ./src/java/org/apache/solr/request/SolrQueryRequest.java ./src/java/org/apache/solr/request/SolrQueryRequestBase.java ./src/java/org/apache/solr/util/CommonParams.java ./client/java/solrj/src/org/apache/solr/client/solrj/request/UpdateRequest.java That should be it, unless I've missed something. Somebody who knows the codebase better than me should have a look over it.
          Hide
          Hoss Man added a comment -

          Reading through Lars's patch, most everything looks fine to me, except...

          1) javadoc generates a lot of warnings like this...

            [javadoc] /home/chrish/lucene/solr/src/java/org/apache/solr/request/SolrQueryRequest.java:107: warning - Tag @link: reference not found: CommonParams#ROWS
            [javadoc] /home/chrish/lucene/solr/src/java/org/apache/solr/request/SolrQueryRequest.java:87: warning - Tag @link: reference not found: CommonParams#Q
            [javadoc] /home/chrish/lucene/solr/src/java/org/apache/solr/request/SolrQueryRequest.java:95: warning - Tag @link: reference not found: CommonParams#QT                                                                             
          ...
          

          ...i think that's just a missing import that we need (didnt' check though)

          2) i'm not really sure about the new isOverwrite() method added to client/java/solrj/src/org/apache/solr/client/solrj/request/UpdateRequest.java ...

          is adding this method the right course of action? what as the intention when deprecating isOverwriteCommitted() and isOverwritePending().

          For that matter: solrJ wasn't in Solr 1.2 ... can't we just outright remove any "@Deprecated" methods from that code?

          Show
          Hoss Man added a comment - Reading through Lars's patch, most everything looks fine to me, except... 1) javadoc generates a lot of warnings like this... [javadoc] /home/chrish/lucene/solr/src/java/org/apache/solr/request/SolrQueryRequest.java:107: warning - Tag @link: reference not found: CommonParams#ROWS [javadoc] /home/chrish/lucene/solr/src/java/org/apache/solr/request/SolrQueryRequest.java:87: warning - Tag @link: reference not found: CommonParams#Q [javadoc] /home/chrish/lucene/solr/src/java/org/apache/solr/request/SolrQueryRequest.java:95: warning - Tag @link: reference not found: CommonParams#QT ... ...i think that's just a missing import that we need (didnt' check though) 2) i'm not really sure about the new isOverwrite() method added to client/java/solrj/src/org/apache/solr/client/solrj/request/UpdateRequest.java ... is adding this method the right course of action? what as the intention when deprecating isOverwriteCommitted() and isOverwritePending(). For that matter: solrJ wasn't in Solr 1.2 ... can't we just outright remove any "@Deprecated" methods from that code?
          Hide
          Ryan McKinley added a comment -

          For that matter: solrJ wasn't in Solr 1.2 ... can't we just outright remove any "@Deprecated" methods from that code?

          +1 — the "overwrite" command is strange because if you use it, you will almost certainly mess things up. I'll make a new issue for that.

          Show
          Ryan McKinley added a comment - For that matter: solrJ wasn't in Solr 1.2 ... can't we just outright remove any "@Deprecated" methods from that code? +1 — the "overwrite" command is strange because if you use it, you will almost certainly mess things up. I'll make a new issue for that.
          Hide
          Lars Kotthoff added a comment -

          i'm not really sure about the new isOverwrite() method added to client/java/solrj/src/org/apache/solr/client/solrj/request/UpdateRequest.java ...

          Actually, neither am I
          I just added it because there was no other way to provide a replacement for the deprecated methods. Furthermore I think you should be able to check for something you can set. It certainly is a strange way to control several booleans with one method though.

          Show
          Lars Kotthoff added a comment - i'm not really sure about the new isOverwrite() method added to client/java/solrj/src/org/apache/solr/client/solrj/request/UpdateRequest.java ... Actually, neither am I I just added it because there was no other way to provide a replacement for the deprecated methods. Furthermore I think you should be able to check for something you can set. It certainly is a strange way to control several booleans with one method though.
          Hide
          Lars Kotthoff added a comment -

          javadoc generates a lot of warnings like this...

          Attaching a new patch against the current TRUNK which fixes this issue. There're more warnings which are not related to this patch, I'll open a new issue for them.

          BTW, what's the policy on using $Id$ keywords in @version tags? It seems that most files don't have them and they make generating patches for anything around that area a real pain.

          Show
          Lars Kotthoff added a comment - javadoc generates a lot of warnings like this... Attaching a new patch against the current TRUNK which fixes this issue. There're more warnings which are not related to this patch, I'll open a new issue for them. BTW, what's the policy on using $Id$ keywords in @version tags? It seems that most files don't have them and they make generating patches for anything around that area a real pain.
          Hide
          Lars Kotthoff added a comment -

          Syncing patch with trunk.

          Show
          Lars Kotthoff added a comment - Syncing patch with trunk.
          Hide
          Grant Ingersoll added a comment -

          +1 on the latest patch. I reviewed that all deprecated tags have notes about alternate usages except DisMaxParams, but not a big deal. I did not review the actual content of each deprecation string, but my perusal suggests they are good enough.

          Show
          Grant Ingersoll added a comment - +1 on the latest patch. I reviewed that all deprecated tags have notes about alternate usages except DisMaxParams, but not a big deal. I did not review the actual content of each deprecation string, but my perusal suggests they are good enough.
          Hide
          Lars Kotthoff added a comment -

          I reviewed that all deprecated tags have notes about alternate usages except DisMaxParams, but not a big deal.

          Whoops, missed that one. Attaching new patch.

          Show
          Lars Kotthoff added a comment - I reviewed that all deprecated tags have notes about alternate usages except DisMaxParams, but not a big deal. Whoops, missed that one. Attaching new patch.
          Hide
          Grant Ingersoll added a comment -

          Sorry, should ask first. Mike, I can take this one if you want...

          Show
          Grant Ingersoll added a comment - Sorry, should ask first. Mike, I can take this one if you want...
          Hide
          Grant Ingersoll added a comment -

          Committed revision 684908.

          Thanks Sean and Lars

          Show
          Grant Ingersoll added a comment - Committed revision 684908. Thanks Sean and Lars

            People

            • Assignee:
              Grant Ingersoll
              Reporter:
              Sean Timm
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development