Http 177: Mark some parts of library Experimental

Http 177: Mark some parts of library Experimental

[yanivi, 2013-03-07 20:22:33]

Great work, Eyal!

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android-t...
File
google-http-client-android-test/src/main/java/com/google/api/client/extensions/android/json/AndroidJsonFactoryTest.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android-t...
google-http-client-android-test/src/main/java/com/google/api/client/extensions/android/json/AndroidJsonFactoryTest.java:27:
@Experimental
Please remove the @Experimental annotation from classes in this project because
it is not a library.  More generally, projects that end in "-test" are not
library projects.

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android/s...
File
google-http-client-android/src/main/java/com/google/api/client/extensions/android/AndroidUtils.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android/s...
google-http-client-android/src/main/java/com/google/api/client/extensions/android/AndroidUtils.java:23:
* Utilities for Android.
As we discussed, please also start the JavaDoc comment of every experimental
public/protected package/class/method with "{@link Experimental} <br/>".  For
example, like this:

/**
 * {@link Experimental} <br/>
 * Utilities for Android.
...

The advantage is that this way it will show up in the global list of
packages/classes/methods in the JavaDoc.  For example:

http://javadoc.google-http-java-client.googlecode.com/hg/1.13.1-beta/index.html
http://javadoc.google-http-java-client.googlecode.com/hg/1.13.1-beta/com/goog...

This is very similar to how @Deprecated is handled in the JavaDoc.  See for
example:

http://javadoc.google-http-java-client.googlecode.com/hg/1.13.1-beta/com/goog...

One exception: don't do this for deprecated package/class/method because when
something is marked Deprecated, the description is not actually shown in the
summary view.   That means you have much fewer files to update.

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android/s...
File
google-http-client-android/src/main/java/com/google/api/client/extensions/android/http/package-info.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android/s...
google-http-client-android/src/main/java/com/google/api/client/extensions/android/http/package-info.java:18:
* <p>
As we discussed, please remove all of the "Warning: this package is..." from all
of the package-info.java files in all of the projects.  Instead we should use a
very similar JavaDoc convention for packages as we do for classes.

First, you have to add ElementType.PACKAGE to the @Target of the @Experimental
annotation.

Then, we should change the JavaDoc here to:

/**
 * {@link com.google.api.client.util.Experimental} <br/>
 * Utilities for Android.
 *
 * @since 1.11
 * @author Yaniv Inbar
 */
@Experimental
package com.google.api.client.extensions.android;

import com.google.api.client.util.Experimental;

https://codereview.appspot.com/7456050/diff/2001/google-http-client-protobuf/...
File
google-http-client-protobuf/src/test/java/com/google/api/client/protobuf/ProtocolBuffersTest.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client-protobuf/...
google-http-client-protobuf/src/test/java/com/google/api/client/protobuf/ProtocolBuffersTest.java:28:
@Experimental
definitely don't want to mark anything under "src/test" as Experimental because
those are test classes.

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:15:
package com.google.api.client.http;
Please also REMOVE the experimental warning from the package-info.java file in
this package and every other package that is no longer experimental.

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockHttpContent.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/MockHttpContent.java:40:
@Experimental
Please don't bother to mark fields/method of experimental classes.  I *do* like
marking sub-classes of experimental classes like Builder classes.

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockHttpTransport.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/MockHttpTransport.java:64:
@Experimental
ditto

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpRequest.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpRequest.java:98:
@Experimental
ditto

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/apache/MockHttpClient.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/apache/MockHttpClient.java:68:
@Experimental
ditto

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/xml/XmlNamespaceDictionary.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/xml/XmlNamespaceDictionary.java:318:
@Experimental
ditto (package-private)

[peleyal, 2013-03-08 16:04:02]

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android-t...
File
google-http-client-android-test/src/main/java/com/google/api/client/extensions/android/json/AndroidJsonFactoryTest.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android-t...
google-http-client-android-test/src/main/java/com/google/api/client/extensions/android/json/AndroidJsonFactoryTest.java:27:
@Experimental
On 2013/03/07 20:22:33, yanivi wrote:
> Please remove the @Experimental annotation from classes in this project
because
> it is not a library.  More generally, projects that end in "-test" are not
> library projects.

Done.

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android/s...
File
google-http-client-android/src/main/java/com/google/api/client/extensions/android/AndroidUtils.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android/s...
google-http-client-android/src/main/java/com/google/api/client/extensions/android/AndroidUtils.java:23:
* Utilities for Android.
On 2013/03/07 20:22:33, yanivi wrote:
> As we discussed, please also start the JavaDoc comment of every experimental
> public/protected package/class/method with "{@link Experimental} <br/>".  For
> example, like this:
> 
> /**
>  * {@link Experimental} <br/>
>  * Utilities for Android.
> ...
> 
> The advantage is that this way it will show up in the global list of
> packages/classes/methods in the JavaDoc.  For example:
> 
>
http://javadoc.google-http-java-client.googlecode.com/hg/1.13.1-beta/index.html
>
http://javadoc.google-http-java-client.googlecode.com/hg/1.13.1-beta/com/goog...
> 
> This is very similar to how @Deprecated is handled in the JavaDoc.  See for
> example:
> 
>
http://javadoc.google-http-java-client.googlecode.com/hg/1.13.1-beta/com/goog...
> 
> One exception: don't do this for deprecated package/class/method because when
> something is marked Deprecated, the description is not actually shown in the
> summary view.   That means you have much fewer files to update.

Done.

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android/s...
File
google-http-client-android/src/main/java/com/google/api/client/extensions/android/http/package-info.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client-android/s...
google-http-client-android/src/main/java/com/google/api/client/extensions/android/http/package-info.java:18:
* <p>
On 2013/03/07 20:22:33, yanivi wrote:
> As we discussed, please remove all of the "Warning: this package is..." from
all
> of the package-info.java files in all of the projects.  Instead we should use
a
> very similar JavaDoc convention for packages as we do for classes.
> 
> First, you have to add ElementType.PACKAGE to the @Target of the @Experimental
> annotation.
> 
> Then, we should change the JavaDoc here to:
> 
> /**
>  * {@link com.google.api.client.util.Experimental} <br/>
>  * Utilities for Android.
>  *
>  * @since 1.11
>  * @author Yaniv Inbar
>  */
> @Experimental
> package com.google.api.client.extensions.android;
> 
> import com.google.api.client.util.Experimental;

Done.

https://codereview.appspot.com/7456050/diff/2001/google-http-client-protobuf/...
File
google-http-client-protobuf/src/test/java/com/google/api/client/protobuf/ProtocolBuffersTest.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client-protobuf/...
google-http-client-protobuf/src/test/java/com/google/api/client/protobuf/ProtocolBuffersTest.java:28:
@Experimental
On 2013/03/07 20:22:33, yanivi wrote:
> definitely don't want to mark anything under "src/test" as Experimental
because
> those are test classes.

Done.

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:15:
package com.google.api.client.http;
it should be in a Different CL. 
On 2013/03/07 20:22:33, yanivi wrote:
> Please also REMOVE the experimental warning from the package-info.java file in
> this package and every other package that is no longer experimental.

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockHttpContent.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/MockHttpContent.java:40:
@Experimental
On 2013/03/07 20:22:33, yanivi wrote:
> Please don't bother to mark fields/method of experimental classes.  I *do*
like
> marking sub-classes of experimental classes like Builder classes.

Done.

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockHttpTransport.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/MockHttpTransport.java:64:
@Experimental
On 2013/03/07 20:22:33, yanivi wrote:
> ditto

Done.

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpRequest.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpRequest.java:98:
@Experimental
On 2013/03/07 20:22:33, yanivi wrote:
> ditto

Done.

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/apache/MockHttpClient.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/apache/MockHttpClient.java:68:
@Experimental
We need that one. It's inside an inner class.
On 2013/03/07 20:22:33, yanivi wrote:
> ditto

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/xml/XmlNamespaceDictionary.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/xml/XmlNamespaceDictionary.java:318:
@Experimental
Can't do. It uses XmlNamespaceDictionary which is Experimental, so it must be
Experimental as well.
Of course I can investigate more the FindBugs plugin - how can we avoid that,
but I don't think it's worth it.
On 2013/03/07 20:22:33, yanivi wrote:
> ditto (package-private)

[yanivi, 2013-03-13 02:26:45]

LGTM

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
(right):

https://codereview.appspot.com/7456050/diff/2001/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:15:
package com.google.api.client.http;
On 2013/03/08 16:04:03, peleyal wrote:
> it should be in a Different CL. 
> On 2013/03/07 20:22:33, yanivi wrote:
> > Please also REMOVE the experimental warning from the package-info.java file
in
> > this package and every other package that is no longer experimental.
> 

[FYI] OK, just please remember to do it soon.   Thanks!

https://codereview.appspot.com/7456050/diff/59008/google-http-client/src/main...
File
google-http-client/src/main/java/com/google/api/client/http/xml/atom/package-info.java
(right):

https://codereview.appspot.com/7456050/diff/59008/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/xml/atom/package-info.java:25:

remove added line

https://codereview.appspot.com/7456050/diff/59008/google-http-client/src/test...
File
google-http-client/src/test/java/com/google/api/client/xml/GenericXmlTest.java
(right):

https://codereview.appspot.com/7456050/diff/59008/google-http-client/src/test...
google-http-client/src/test/java/com/google/api/client/xml/GenericXmlTest.java:18:

[optional] please revert changes to this file so we can keep this changeset more
narrowly focused to the task at hand which is the Experimental attribute.

[peleyal, 2013-03-13 14:57:05]

Done

https://codereview.appspot.com/7456050/diff/59008/google-http-client/src/main...
File
google-http-client/src/main/java/com/google/api/client/http/xml/atom/package-info.java
(right):

https://codereview.appspot.com/7456050/diff/59008/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/xml/atom/package-info.java:25:

On 2013/03/13 02:26:45, yanivi wrote:
> remove added line

Done.

https://codereview.appspot.com/7456050/diff/59008/google-http-client/src/test...
File
google-http-client/src/test/java/com/google/api/client/xml/GenericXmlTest.java
(right):

https://codereview.appspot.com/7456050/diff/59008/google-http-client/src/test...
google-http-client/src/test/java/com/google/api/client/xml/GenericXmlTest.java:18:

On 2013/03/13 02:26:45, yanivi wrote:
> [optional] please revert changes to this file so we can keep this changeset
more
> narrowly focused to the task at hand which is the Experimental attribute.

Done.