Skip to content

Adding C++ API and implementation for STS credentials:#19587

Merged
jboeuf merged 1 commit intogrpc:masterfrom
jboeuf:sts_creds_json
Jul 10, 2019
Merged

Adding C++ API and implementation for STS credentials:#19587
jboeuf merged 1 commit intogrpc:masterfrom
jboeuf:sts_creds_json

Conversation

@jboeuf
Copy link
Copy Markdown
Contributor

@jboeuf jboeuf commented Jul 8, 2019

  • marked as experimental.
  • also changed the name of a field in the options struct.

@jboeuf jboeuf added lang/c++ area/security release notes: no Indicates if PR should not be in release notes labels Jul 8, 2019
@jboeuf jboeuf requested review from jiangtaoli2016 and yang-g July 9, 2019 05:23
@jboeuf
Copy link
Copy Markdown
Contributor Author

jboeuf commented Jul 9, 2019

Note, this is the follow up of #19032 which added the STS creds functionality in core and that was reviewed by @markdroth

/cc @JimmyCYJ

Copy link
Copy Markdown
Contributor

@yang-g yang-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with minor nits.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use <> for public headers?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I don't need this one since it's already on the corresponding header.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use <> for public headers?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Copy Markdown

@jiangtaoli2016 jiangtaoli2016 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

- marked as experimental.
- also changed the name of a field in the options struct.
@jboeuf
Copy link
Copy Markdown
Contributor Author

jboeuf commented Jul 10, 2019

Thanks much for the quick reviews @yang-g and @jiangtaoli2016.
Rebasing and squashing before merging.

@jboeuf jboeuf merged commit 59334d8 into grpc:master Jul 10, 2019
@jboeuf jboeuf deleted the sts_creds_json branch July 10, 2019 10:55
@lock lock bot locked as resolved and limited conversation to collaborators Oct 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area/security lang/c++ release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants