|
|
Created:
9 years, 10 months ago by peleyal Modified:
9 years, 9 months ago CC:
google-api-dotnet-client_google.com Base URL:
https://google-api-dotnet-client.googlecode.com/hg/ Visibility:
Public. |
DescriptionImplementation for ComputeCredential.
srashid@ already tested that it works.
TODO(peleyal): Verify the our G sample works with the changed to ServiceAccountCredential.
Patch Set 1 #Patch Set 2 : comments #Patch Set 3 : minor #
Total comments: 16
Patch Set 4 : minor #Patch Set 5 : minor minor #Patch Set 6 : reverting changes to Program release #
MessagesTotal messages: 5
Hi, Note: Most of the content of ComputeCredential was copied from ServiceAccountCredential because it is being shared between compute and service account now.
Sign in to reply to this message.
A few nits and comment change suggestions https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... File Src/GoogleApis.Auth/OAuth2/ComputeCredential.cs (right): https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ComputeCredential.cs:55: : this(new Initializer()) { } Is there a reason you added the newline before ' : ' operator here and below? You should be able to put this on the previous line https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... File Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs (right): https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:31: /// Google OAuth 2.0 credential for accessing protected resources using an access token. The Google OAuth 2.0 Maybe rephrase the first paragraph of the summary as: "" This type of Google OAuth 2.0 credential enables access to protected resources using an access token when interacting server to server. For example, a service account credential could be used to access Google Cloud Storage from a web application without a user's involvement. "" https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:36: /// Take a look at <see cref="Google.Apis.Auth.OAuth2.ServiceAccountCredential"/>. Rephrase "Take a look at" ... as: See also: <see cref="Google.Apis.Auth.OAuth2.ServiceAccountCredential" /> https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:40: /// class in order to support Compute credentials. More further reading about Compute authentication, take a look "More further reading about Compute authentication," better rephrased as: For more information about Compute authentication, see: https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:56: /// Gets or sets the clock. The clock is used to determine if the token has expired, if so we will try to Gets or sets the clock used to refresh the token when it expires. The default value is ... https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:106: /// Gets the clock. The clock is used to determine if the token has expired, if so we will try to refresh it. Gets the clock used to refresh the token if it expires. https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:139: /// <param name="initializer"></param> Missing parameter description
Sign in to reply to this message.
https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... File Src/GoogleApis.Auth/OAuth2/ComputeCredential.cs (right): https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ComputeCredential.cs:55: : this(new Initializer()) { } On 2014/12/11 18:39:10, class wrote: > Is there a reason you added the newline before ' : ' operator here and below? > You should be able to put this on the previous line That's what happened when I used the default formatter. Do you think I should change it? It doesn't really important to me - so whatever you decided :) https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... File Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs (right): https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:31: /// Google OAuth 2.0 credential for accessing protected resources using an access token. The Google OAuth 2.0 On 2014/12/11 18:39:11, class wrote: > Maybe rephrase the first paragraph of the summary as: > "" > This type of Google OAuth 2.0 credential enables access to protected resources > using an access token when interacting server to server. > For example, a service account credential could be used to access Google Cloud > Storage from a web application without a user's involvement. > "" Done. https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:36: /// Take a look at <see cref="Google.Apis.Auth.OAuth2.ServiceAccountCredential"/>. On 2014/12/11 18:39:10, class wrote: > Rephrase "Take a look at" ... as: > > See also: <see cref="Google.Apis.Auth.OAuth2.ServiceAccountCredential" /> Done. https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:40: /// class in order to support Compute credentials. More further reading about Compute authentication, take a look On 2014/12/11 18:39:11, class wrote: > "More further reading about Compute authentication," > > better rephrased as: > > For more information about Compute authentication, see: Done. https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:56: /// Gets or sets the clock. The clock is used to determine if the token has expired, if so we will try to On 2014/12/11 18:39:10, class wrote: > Gets or sets the clock used to refresh the token when it expires. The default > value is ... Done. https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:106: /// Gets the clock. The clock is used to determine if the token has expired, if so we will try to refresh it. On 2014/12/11 18:39:11, class wrote: > Gets the clock used to refresh the token if it expires. Done. https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ServiceCredential.cs:139: /// <param name="initializer"></param> On 2014/12/11 18:39:10, class wrote: > Missing parameter description Done.
Sign in to reply to this message.
LGTM, optionally remove the extra newlines on the initializers passed in to the ComputeCredential constructors. https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... File Src/GoogleApis.Auth/OAuth2/ComputeCredential.cs (right): https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ComputeCredential.cs:55: : this(new Initializer()) { } On 2014/12/12 23:13:29, peleyal wrote: > On 2014/12/11 18:39:10, class wrote: > > Is there a reason you added the newline before ' : ' operator here and below? > > You should be able to put this on the previous line > > That's what happened when I used the default formatter. > Do you think I should change it? > > It doesn't really important to me - so whatever you decided :) I don't feel too strongly but : on the same line seems cleaner...
Sign in to reply to this message.
pushing now :) https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... File Src/GoogleApis.Auth/OAuth2/ComputeCredential.cs (right): https://codereview.appspot.com/181560043/diff/40001/Src/GoogleApis.Auth/OAuth... Src/GoogleApis.Auth/OAuth2/ComputeCredential.cs:55: : this(new Initializer()) { } On 2014/12/12 23:27:57, class wrote: > On 2014/12/12 23:13:29, peleyal wrote: > > On 2014/12/11 18:39:10, class wrote: > > > Is there a reason you added the newline before ' : ' operator here and > below? > > > You should be able to put this on the previous line > > > > That's what happened when I used the default formatter. > > Do you think I should change it? > > > > It doesn't really important to me - so whatever you decided :) > > I don't feel too strongly but : on the same line seems cleaner... Done.
Sign in to reply to this message.
|