Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(4218)

Issue 6242068: [oauth] FileCredentialStore implementation of CredentialStore interface (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 10 months ago by rafael.naufal
Modified:
1 year, 9 months ago
Base URL:
https://google-oauth-java-client.googlecode.com/hg/
Visibility:
Public.

Description

[oauth] FileCredentialStore implementation of CredentialStore interface

Patch Set 1 #

Total comments: 21

Patch Set 2 : Applying comments from reviewers [yanivi, rmistry] #

Patch Set 3 : Applying comments from reviewers [yanivi, rmistry] #

Total comments: 2

Patch Set 4 : Creating new project google-oauth-client-java6 #

Total comments: 2

Patch Set 5 : Changing the changeset to 1.11 #

Patch Set 6 : Changing the changeset to 1.11 #

Total comments: 15

Patch Set 7 : Applying latest comments from [rmistry] #

Total comments: 43

Patch Set 8 : Applying latest comments from [yanivi] and adding dependencies.html #

Total comments: 19

Patch Set 9 : Latest comments from [yanivi] #

Patch Set 10 : Latest comments from [yanivi] #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+633 lines, -32 lines) Patch
M google-oauth-client-appengine/pom.xml View 1 2 3 4 5 6 7 1 chunk +29 lines, -16 lines 0 comments Download
M google-oauth-client-assembly/pom.xml View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
A google-oauth-client-java6/.classpath View 1 2 3 4 5 6 7 8 1 chunk +8 lines, -0 lines 0 comments Download
A google-oauth-client-java6/.project View 1 2 3 4 5 6 7 8 1 chunk +18 lines, -0 lines 0 comments Download
A google-oauth-client-java6/pom.xml View 1 2 3 4 5 6 1 chunk +69 lines, -0 lines 0 comments Download
A google-oauth-client-java6/resources/credentials.json View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
A google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FileCredentialStore.java View 1 2 3 4 5 6 7 8 1 chunk +124 lines, -0 lines 0 comments Download
A google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FilePersistedCredential.java View 1 2 3 4 5 6 7 8 1 chunk +71 lines, -0 lines 0 comments Download
A google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FilePersistedCredentials.java View 1 2 3 4 5 6 7 8 1 chunk +85 lines, -0 lines 2 comments Download
A google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/package-info.java View 1 2 3 4 5 6 7 1 chunk +27 lines, -0 lines 0 comments Download
A google-oauth-client-java6/src/test/java/com/google/api/client/extensions/java6/auth/oauth2/FileCredentialStoreTest.java View 1 2 3 4 5 6 7 8 1 chunk +156 lines, -0 lines 0 comments Download
A google-oauth-client-java6/src/test/resources/credentials.json View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M google-oauth-client-servlet/pom.xml View 1 2 3 4 5 6 7 1 chunk +29 lines, -16 lines 0 comments Download
M jdiff.xml View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M pom.xml View 1 2 3 4 5 6 3 chunks +10 lines, -0 lines 0 comments Download

Messages

Total messages: 39
rafael.naufal
1 year, 10 months ago #1
yanivi
Thanks so much for sending this for review. We really appreciate your contribution. Here are ...
1 year, 10 months ago #2
rafael.naufal
Thanks, yanivi. I'll apply your comments and submit my corrections ASAP. On 2012/05/30 18:46:18, yanivi ...
1 year, 10 months ago #3
rmistry
http://codereview.appspot.com/6242068/diff/1/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java File google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java (right): http://codereview.appspot.com/6242068/diff/1/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java#newcode67 google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java:67: throw new RuntimeException("Cannot store credentials on file: ", e); ...
1 year, 10 months ago #4
yanivi
http://codereview.appspot.com/6242068/diff/1/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java File google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java (right): http://codereview.appspot.com/6242068/diff/1/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java#newcode35 google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java:35: public class FileCredentialStore implements CredentialStore { please add unit ...
1 year, 10 months ago #5
rafael.naufal
1 year, 10 months ago #6
rafael.naufal
1 year, 10 months ago #7
rafael.naufal
http://codereview.appspot.com/6242068/diff/1/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java File google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java (right): http://codereview.appspot.com/6242068/diff/1/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java#newcode35 google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java:35: public class FileCredentialStore implements CredentialStore { On 2012/05/31 12:54:14, ...
1 year, 10 months ago #8
rafael.naufal
1 year, 10 months ago #9
jcgregorio_google
http://codereview.appspot.com/6242068/diff/9002/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java File google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java (right): http://codereview.appspot.com/6242068/diff/9002/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java#newcode68 google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java:68: .exec(new String[] { "chmod", "600", chmod won't work under ...
1 year, 10 months ago #10
yanivi
http://codereview.appspot.com/6242068/diff/9002/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java File google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java (right): http://codereview.appspot.com/6242068/diff/9002/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java#newcode68 google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/FileCredentialStore.java:68: .exec(new String[] { "chmod", "600", On 2012/06/04 13:11:53, jcgregorio_google ...
1 year, 10 months ago #11
rafael.naufal
So I have to create another artifact/jar called google-oauth-client-java6 and move FileCredentialStore into it? Do ...
1 year, 10 months ago #12
yanivi
On 2012/06/04 15:00:26, rafael.naufal wrote: > So I have to create another artifact/jar called google-oauth-client-java6 ...
1 year, 10 months ago #13
rafael.naufal
Ok, just one doubt: if the file doesn't exist (the else clause) on the constructor, ...
1 year, 10 months ago #14
rafael.naufal
1 year, 10 months ago #15
yanivi
Sorry, I'm too busy to review in detail this week, but I wanted to give ...
1 year, 10 months ago #16
rafael.naufal
1 year, 10 months ago #17
rafael.naufal
1 year, 10 months ago #18
rafael.naufal
http://codereview.appspot.com/6242068/diff/21001/google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FileCredentialStore.java File google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FileCredentialStore.java (right): http://codereview.appspot.com/6242068/diff/21001/google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FileCredentialStore.java#newcode34 google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FileCredentialStore.java:34: * @since 1.10 Ok, done! On 2012/06/06 18:53:01, yanivi ...
1 year, 10 months ago #19
yanivi
Sorry I haven't been able to find time to review this just because I'm preparing ...
1 year, 10 months ago #20
rmistry
http://codereview.appspot.com/6242068/diff/24002/google-oauth-client-appengine/.classpath File google-oauth-client-appengine/.classpath (right): http://codereview.appspot.com/6242068/diff/24002/google-oauth-client-appengine/.classpath#newcode1 google-oauth-client-appengine/.classpath:1: <classpath> As Yaniv mentioned before for google-oauth-client-appengine and google-oauth-client-servlet: ...
1 year, 10 months ago #21
rafael.naufal
1 year, 9 months ago #22
rafael.naufal
http://codereview.appspot.com/6242068/diff/24002/google-oauth-client-java6/pom.xml File google-oauth-client-java6/pom.xml (right): http://codereview.appspot.com/6242068/diff/24002/google-oauth-client-java6/pom.xml#newcode1 google-oauth-client-java6/pom.xml:1: <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" On 2012/06/20 13:45:39, rmistry wrote: > ...
1 year, 9 months ago #23
yanivi
Thanks for your patience Rafael! I'm back from Google I/O and I've finally had a ...
1 year, 9 months ago #24
rafael.naufal
Hi Yanivi, I applied all your latest comments, but I have one doubt regarding the ...
1 year, 9 months ago #25
yanivi
On 2012/07/04 21:30:30, rafael.naufal wrote: > Hi Yanivi, > > I applied all your latest ...
1 year, 9 months ago #26
rafael.naufal
Ok, thanks Yanivi! The files you mentioned on your last email are created with Maven ...
1 year, 9 months ago #27
yanivi
Ideally I'd like you to update them all now as part of this changeset (nothing ...
1 year, 9 months ago #28
rafael.naufal
How can I generate this *-dependencies.html file? On Mon, Jul 9, 2012 at 4:41 PM, ...
1 year, 9 months ago #29
rmistry
On 2012/07/10 20:39:40, rafael.naufal wrote: > How can I generate this *-dependencies.html file? > Details ...
1 year, 9 months ago #30
rafael.naufal
1 year, 9 months ago #31
rafael.naufal
http://codereview.appspot.com/6242068/diff/36001/google-oauth-client-java6/.classpath File google-oauth-client-java6/.classpath (right): http://codereview.appspot.com/6242068/diff/36001/google-oauth-client-java6/.classpath#newcode4 google-oauth-client-java6/.classpath:4: <classpathentry kind="src" path="src/main/java" including="**/*.java"/> On 2012/07/03 19:41:13, yanivi wrote: ...
1 year, 9 months ago #32
yanivi
Only a few minor comments left, so we are very close to this being committed. ...
1 year, 9 months ago #33
rafael.naufal
1 year, 9 months ago #34
rafael.naufal
1 year, 9 months ago #35
rafael.naufal
http://codereview.appspot.com/6242068/diff/41003/google-oauth-client-java6/.classpath File google-oauth-client-java6/.classpath (right): http://codereview.appspot.com/6242068/diff/41003/google-oauth-client-java6/.classpath#newcode7 google-oauth-client-java6/.classpath:7: path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.6" /> On 2012/07/11 14:58:15, yanivi wrote: > please ...
1 year, 9 months ago #36
yanivi
LGTM Congrats! Please commit using the directions below: http://code.google.com/p/google-oauth-java-client/wiki/BecomingAContributor#Committing_the_code Let me know if you run ...
1 year, 9 months ago #37
rafael.naufal
Changes committed! http://code.google.com/p/google-oauth-java-client/source/browse/#hg%2Fgoogle-oauth-client-java6 http://codereview.appspot.com/6242068/diff/54006/google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FilePersistedCredentials.java File google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FilePersistedCredentials.java (right): http://codereview.appspot.com/6242068/diff/54006/google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FilePersistedCredentials.java#newcode37 google-oauth-client-java6/src/main/java/com/google/api/client/extensions/java6/auth/oauth2/FilePersistedCredentials.java:37: new HashMap<String, FilePersistedCredential>(); On 2012/07/12 18:22:31, ...
1 year, 9 months ago #38
yanivi
1 year, 9 months ago #39
LGTM

Please officially close this issue by clicking on the "x in a circle" icon to
the left of the word "Issue".
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1278:e6ce13d99bf5