Update samples to use new generated library based on 1.5

Update Buzz, Discovery, & Prediction samples to use new generated library based on 1.5

[yanivi, 2011-07-11 12:59:12]

Ping.

[aiuto, 2011-07-11 14:37:45]

more comments to follow

There's something going wrong between codereview and my browser, making some
things impossible to view side by side to enter comments.

My big concern is that we're going to update the same few files in identical
ways in each CL, particularly  the Debug and Auth stuff.

We'ld probably be better off keeping them in only one place (package rename
required) and having the sync tool distribute to multiple samples. That will
save review time and minimize the chance of divergence.

http://codereview.appspot.com/4654066/diff/3001/buzz-v1-json-oauth2-cmdline-s...
File buzz-v1-json-oauth2-cmdline-sample/checkstyle-suppressions.xml (right):

http://codereview.appspot.com/4654066/diff/3001/buzz-v1-json-oauth2-cmdline-s...
buzz-v1-json-oauth2-cmdline-sample/checkstyle-suppressions.xml:1: <?xml
version="1.0"?>
Why do we have this?  Adding extra files which don't illustrate the library
usage adds mental clutter.

I'ld drop it.

http://codereview.appspot.com/4654066/diff/3001/buzz-v1-json-oauth2-cmdline-s...
File buzz-v1-json-oauth2-cmdline-sample/pom.xml (left):

http://codereview.appspot.com/4654066/diff/3001/buzz-v1-json-oauth2-cmdline-s...
buzz-v1-json-oauth2-cmdline-sample/pom.xml:102:
<groupId>com.google.api.client</groupId>
Don't you invoke the client directly to create the request factories.  You
should include what you use.

http://codereview.appspot.com/4654066/diff/3001/checkstyle.xml
File checkstyle.xml (right):

http://codereview.appspot.com/4654066/diff/3001/checkstyle.xml#newcode1
checkstyle.xml:1: <?xml version="1.0" encoding="UTF-8"?>
Let''s not clutter the examples with this stuff. It's completely orthogonal to
the issue of showing people how to use the libraries.

[yanivi, 2011-07-11 17:23:03]

I like your idea of factoring out stuff like the Auth code into a single place,
though I would strongly argue if we do that, that it should be that way
externally, not just internally.  But I see this as something to think about
long-term, not necessarily a blocker for this release.  For right now, I'm
narrowly focused on updating the samples to the 1.5 release.

http://codereview.appspot.com/4654066/diff/3001/buzz-v1-json-oauth2-cmdline-s...
File buzz-v1-json-oauth2-cmdline-sample/checkstyle-suppressions.xml (right):

http://codereview.appspot.com/4654066/diff/3001/buzz-v1-json-oauth2-cmdline-s...
buzz-v1-json-oauth2-cmdline-sample/checkstyle-suppressions.xml:1: <?xml
version="1.0"?>
On 2011/07/11 14:37:45, aiuto wrote:
> Why do we have this?  Adding extra files which don't illustrate the library
> usage adds mental clutter.
> 
> I'ld drop it.

See the other comment.

http://codereview.appspot.com/4654066/diff/3001/buzz-v1-json-oauth2-cmdline-s...
File buzz-v1-json-oauth2-cmdline-sample/pom.xml (left):

http://codereview.appspot.com/4654066/diff/3001/buzz-v1-json-oauth2-cmdline-s...
buzz-v1-json-oauth2-cmdline-sample/pom.xml:102:
<groupId>com.google.api.client</groupId>
On 2011/07/11 14:37:45, aiuto wrote:
> Don't you invoke the client directly to create the request factories.  You
> should include what you use.

Done (also in the other pom.xml).

http://codereview.appspot.com/4654066/diff/3001/checkstyle.xml
File checkstyle.xml (right):

http://codereview.appspot.com/4654066/diff/3001/checkstyle.xml#newcode1
checkstyle.xml:1: <?xml version="1.0" encoding="UTF-8"?>
On 2011/07/11 14:37:45, aiuto wrote:
> Let''s not clutter the examples with this stuff. It's completely orthogonal to
> the issue of showing people how to use the libraries.

Showing people the best practices for setting up projects that use our client
libraries is within the scope of what we should be doing in our samples.  Even
more importantly, we should enforce good coding style in our samples, which this
checkstyle file helps achieve.  I agree that the extra clutter is unfortunate,
but I think it is sufficiently minimal to be outweighed by the benefits.

[aiuto, 2011-07-11 20:33:11]

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/google
File discovery-sample/google (right):

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/google#newco...
discovery-sample/google:18: #
http://code.google.com/p/oacurl/source/browse/oacurl
This comment belongs in the resolve_symlink function, but if you replace it, you
can remove it.

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/google#newco...
discovery-sample/google:20: function resolve_symlink {
Yuck. This is crazy inefficient and baroque.

How about....

resolve_symlink()
{
  case $1 in
  /* )
   PROGPATH="$1"
   ;;
  * )
   PROGPATH=$(pwd)/"$1"
   ;;
  esac
  echo $(cd $(dirname $PROGPATH) && /bin/pwd)
}

I've been using that for years.

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/google#newco...
discovery-sample/google:65: $JAVACMD -jar
$DISCOVERY_HOME/target/discovery-sample-1.2.0-jar-with-dependencies.jar $@
Why is this file called 'google'?  It should probably be 'run_sample' or
something indicative of what it does.

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/src/com/goog...
File
discovery-sample/src/com/google/api/client/sample/discovery/oauth/CredentialStore.java
(right):

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/src/com/goog...
discovery-sample/src/com/google/api/client/sample/discovery/oauth/CredentialStore.java:31:
*     
"http://developer.apple.com/library/mac/#documentation/Security/Conceptual/keychainServConcepts/01introduction/introduction.html"
You might replace these with searches to make them less brittle against Apple
changing their dev site.  E.g.

http://www.google.com/webhp?q=OS%2FX+Keychain+service+concepts+introduction

Then it might fit in 100 characters as well.

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/src/com/goog...
discovery-sample/src/com/google/api/client/sample/discovery/oauth/CredentialStore.java:35:
public interface CredentialStore {
Does anything in this CL implement this? I can't find it?

[yanivi, 2011-08-05 19:33:09]

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/google
File discovery-sample/google (right):

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/google#newco...
discovery-sample/google:18: #
http://code.google.com/p/oacurl/source/browse/oacurl
On 2011/07/11 20:33:11, aiuto wrote:
> This comment belongs in the resolve_symlink function, but if you replace it,
you
> can remove it.

Done.

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/google#newco...
discovery-sample/google:20: function resolve_symlink {
On 2011/07/11 20:33:11, aiuto wrote:
> Yuck. This is crazy inefficient and baroque.
> 
> How about....
> 
> resolve_symlink()
> {
>   case $1 in
>   /* )
>    PROGPATH="$1"
>    ;;
>   * )
>    PROGPATH=$(pwd)/"$1"
>    ;;
>   esac
>   echo $(cd $(dirname $PROGPATH) && /bin/pwd)
> }
> 
> I've been using that for years.

Done.

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/google#newco...
discovery-sample/google:65: $JAVACMD -jar
$DISCOVERY_HOME/target/discovery-sample-1.2.0-jar-with-dependencies.jar $@
On 2011/07/11 20:33:11, aiuto wrote:
> Why is this file called 'google'?  It should probably be 'run_sample' or
> something indicative of what it does.

Done.

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/src/com/goog...
File
discovery-sample/src/com/google/api/client/sample/discovery/oauth/CredentialStore.java
(right):

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/src/com/goog...
discovery-sample/src/com/google/api/client/sample/discovery/oauth/CredentialStore.java:31:
*     
"http://developer.apple.com/library/mac/#documentation/Security/Conceptual/keychainServConcepts/01introduction/introduction.html"
On 2011/07/11 20:33:11, aiuto wrote:
> You might replace these with searches to make them less brittle against Apple
> changing their dev site.  E.g.
> 
> http://www.google.com/webhp?q=OS%252FX+Keychain+service+concepts+introduction
> 
> Then it might fit in 100 characters as well.

Done.

http://codereview.appspot.com/4654066/diff/9001/discovery-sample/src/com/goog...
discovery-sample/src/com/google/api/client/sample/discovery/oauth/CredentialStore.java:35:
public interface CredentialStore {
On 2011/07/11 20:33:11, aiuto wrote:
> Does anything in this CL implement this? I can't find it?

No.  JavaDoc comment added.

[rmistry, 2011-08-08 17:39:42]

http://codereview.appspot.com/4654066/diff/17001/buzz-cmdline-sample/.classpath
File buzz-cmdline-sample/.classpath (right):

http://codereview.appspot.com/4654066/diff/17001/buzz-cmdline-sample/.classpa...
buzz-cmdline-sample/.classpath:1: <?xml version="1.0" encoding="UTF-8"?>
Can we remove files from the CL with no changes in them? Or is there a reason
they were included?

http://codereview.appspot.com/4654066/diff/17001/buzz-cmdline-sample/src/main...
File
buzz-cmdline-sample/src/main/java/com/google/api/services/samples/buzz/cmdline/GroupActions.java
(right):

http://codereview.appspot.com/4654066/diff/17001/buzz-cmdline-sample/src/main...
buzz-cmdline-sample/src/main/java/com/google/api/services/samples/buzz/cmdline/GroupActions.java:65:

nitpick: Unnecessary newline.

[rmistry, 2011-08-08 18:07:30]

LGTM

[yanivi, 2011-08-08 19:37:31]

committing now...

http://codereview.appspot.com/4654066/diff/17001/buzz-cmdline-sample/.classpath
File buzz-cmdline-sample/.classpath (right):

http://codereview.appspot.com/4654066/diff/17001/buzz-cmdline-sample/.classpa...
buzz-cmdline-sample/.classpath:1: <?xml version="1.0" encoding="UTF-8"?>
On 2011/08/08 17:39:45, rmistry wrote:
> Can we remove files from the CL with no changes in them? Or is there a reason
> they were included?

These files are simply moved.

http://codereview.appspot.com/4654066/diff/17001/buzz-cmdline-sample/src/main...
File
buzz-cmdline-sample/src/main/java/com/google/api/services/samples/buzz/cmdline/GroupActions.java
(right):

http://codereview.appspot.com/4654066/diff/17001/buzz-cmdline-sample/src/main...
buzz-cmdline-sample/src/main/java/com/google/api/services/samples/buzz/cmdline/GroupActions.java:65:

On 2011/08/08 17:39:45, rmistry wrote:
> nitpick: Unnecessary newline.

Done.