Skip to content
This repository was archived by the owner on Jan 18, 2025. It is now read-only.

Conversation

@dlorenc
Copy link
Contributor

@dlorenc dlorenc commented Jan 19, 2016

This adds to_json and from_json methods to GoogleCredentials
and _ServiceAccountCredentials classes. This allows them to
be serialized by multistore_file.

Resovles: #384

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@googlebot
Copy link

CLAs look good, thanks!

@dlorenc
Copy link
Contributor Author

dlorenc commented Jan 19, 2016

Thanks for the review, addressed most of the comments and pushed a new commit.

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor

dhermes commented Jan 22, 2016

@nathanielmanistaatgoogle PTAL

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor

dhermes commented Jan 22, 2016

This LGTM but would like to give @nathanielmanistaatgoogle a chance to make a pass.

@nathanielmanistaatgoogle
Copy link
Contributor

"Resovles" in your commit message is a typo and should be "Resolves", right?

@dhermes
Copy link
Contributor

dhermes commented Jan 22, 2016

Yeah resolves is in https://help.github.com/articles/closing-issues-via-commit-messages/ but Resovles is not 😀

@dlorenc
Copy link
Contributor Author

dlorenc commented Jan 22, 2016

Thanks for catching the typo. Fixed.

@nathanielmanistaatgoogle
Copy link
Contributor

(Will have other comments later; still working through the content of this change.)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@dlorenc
Copy link
Contributor Author

dlorenc commented Jan 23, 2016

Addressed feedback. PTAL @nathanielmanistaatgoogle

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@nathanielmanistaatgoogle
Copy link
Contributor

Aside from what we're discussing about the TODO comment I'm happy with the content of this change. Please squash commits - a code review is just a code review; there's no need for this change to be more than one piece of project history (two after GitHub does its thing).

This adds to_json and from_json methods to GoogleCredentials
and _ServiceAccountCredentials classes. This allows them to
be serialized by multistore_file.

Resolves: googleapis#384
@dlorenc
Copy link
Contributor Author

dlorenc commented Jan 27, 2016

Added the TODO and squashed. Thanks for the review!

@nathanielmanistaatgoogle
Copy link
Contributor

Looks good; only waiting for tests to repass.

nathanielmanistaatgoogle added a commit that referenced this pull request Jan 27, 2016
Add to/from json methods to Credentials classes.
@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit 0b6e72e into googleapis:master Jan 27, 2016
@dhermes dhermes mentioned this pull request Jan 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants