Skip to content
This repository was archived by the owner on Aug 19, 2019. It is now read-only.

Conversation

@igorpeshansky
Copy link
Contributor

No description provided.

namespace {

json::value ReadCredentials(const std::string& credentials_file) throw(json::Exception) {
class NoCredentials {

Choose a reason for hiding this comment

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

The spacing for this class seems unusual. Is that on purpose? Also, how about naming this NoCredentialsException?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just an informational class to convey a specific (non-exceptional) status. The code catches and ignores it, modulo info logging.
What's unusual about the spacing?

Choose a reason for hiding this comment

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

Per conversation offline, this is the expected formatting. public and private should have one-space indentation per convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

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

LGTM.

namespace {

json::value ReadCredentials(const std::string& credentials_file) throw(json::Exception) {
class NoCredentials {

Choose a reason for hiding this comment

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

Per conversation offline, this is the expected formatting. public and private should have one-space indentation per convention.

Copy link
Contributor

@bmoyles0117 bmoyles0117 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@igorpeshansky igorpeshansky merged commit 14bce33 into master Sep 14, 2017
@igorpeshansky igorpeshansky deleted the igorp-optional-credentials branch September 14, 2017 23:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants