-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support file-based properties #51
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looking good. Thank you!
Left 1 comment. PTAL!
properties['scalar.dl.client.tls.ca_root_cert_pem'] = | ||
properties['scalar.dl.client.tls.ca_root_cert_pem'] || | ||
fs.readFileSync( | ||
properties['scalar.dl.client.tls.ca_root_cert_path'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need ,
at the end?
Also, should this be formatted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that is because we use Google's ESLint config and it has this rule (comma-dangle)
https://github.com/google/eslint-config-google/blob/master/index.js#L195
It not only applies to the array and the object but also to the function arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is formatted properly, It's fine!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
properties['scalar.dl.client.tls.ca_root_cert_pem'] = | ||
properties['scalar.dl.client.tls.ca_root_cert_pem'] || | ||
fs.readFileSync( | ||
properties['scalar.dl.client.tls.ca_root_cert_path'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is formatted properly, It's fine!
This PR makes ClientService support the following properties
Developers can configure keys and certificates by giving related file paths.