-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
support 'false' and 'true' values of 'encryption' parameter (#160)
- Loading branch information
Showing
1 changed file
with
2 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
939dda0
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.
@aler9 I see that encryption is defined as a
string
in theConf struct
. Any particular reason for not usingbool
like all the other parameters? Because this commit doesn't really fix the bug, as it will still fail with valid YAML values like True, TRUE or Yes, which other yaml serializers might dump.939dda0
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.
@dorinclisu encryption must also accept the "optional" value, so it can't be stored in a bool - i should have chosen the "strict" value instead of "yes" to make it more clear
939dda0
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.
Perhaps a better way to handle "optional" is to still keep encryption boolean but have its presence optional. So if the field is not present in the config file, it is treated as optional, if it is present then it's treated as strict yes/no.