Add defaults to config cheatsheet, and minor language cleanup. #3290
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3290 +/- ##
==========================================
+ Coverage 34.67% 34.86% +0.19%
==========================================
Files 278 278
Lines 40506 40506
==========================================
+ Hits 14044 14124 +80
+ Misses 24395 24298 -97
- Partials 2067 2084 +17
Continue to review full report at Codecov.
|
tboerger
added
the
lgtm/need 2
| - `WHITELISTED_URIS`: Space separated list of POSIX regexp patterns. If non empty OpenID URIs should match any of these to be granted access. | ||
| - `BLACKLISTED_URIS`: Space separated list of POSIX regexp pattenrs. OpenID URI matching any of these is refused access. | ||
| - `ENABLE_OPENID_SIGNIN`: **false**: Allow authentication in via OpenID. | ||
| - `ENABLE_OPENID_SIGNUP`: **! DISABLE\_REGISTRATION**: Allow registering via OpenID. |
There was a problem hiding this comment.
here the exclamation mark has nothing to do with the importance of knowing what you're doing. Rather it's negating the DISABLE_REGISTRATION setting. I'm mentioning becuase I found it weird to read that meaning of the mark at the top of the file
There was a problem hiding this comment.
I think it might make the most sense to remove the use of :exclamation: since it's only used for two settings, and the mark doesn't actually render anyway. Those two options can easily have a small note added about not fiddling with them.
There was a problem hiding this comment.
you didn't get it: it's nothing about the risk in fiddling with them. Exclamation mark only means NEGATING the value of the following value. So ENABLE_OPENID_SIGNUP is FALSE when DISABLE_REGISTRATION is TRUE and vice-versa.
| - `ENABLE_OPENID_SIGNIN`: **false**: Allow authentication in via OpenID. | ||
| - `ENABLE_OPENID_SIGNUP`: **! DISABLE\_REGISTRATION**: Allow registering via OpenID. | ||
| - `WHITELISTED_URIS`: **\<none\>**: If non-empty, list of POSIX regex patterns matching OpenID URI's to permit. | ||
| - `BLACKLISTED_URIS`: **\<none\>**: If non-empty, list of POSIX regex pattenrs matching OpenID URI's to block. |
There was a problem hiding this comment.
also if empty, would be the empty list.... This is different from WHITELISTED_URL in that an empty list would NOT mean to allow NOBODY but rather to allow ALL.
There was a problem hiding this comment.
Is this just a problem with using the <none>? Would it be better to s/none/empty/?
There was a problem hiding this comment.
It's a problem with the wording If non-empty, which is not needed for BLACKLISTED_URIS. There's no difference in behavior when the value is EMPTY. Also please mind the typos
| - `LEVEL`: General log level, default is `Trace`. | ||
| - `ROOT_PATH`: **\<none\>**: Root path for log files. | ||
| - `MODE`: **console**: Logging mode. For multiple modes, use a comma to separate values. | ||
| - `LEVEL`: **Trace**: General log level. |
| @@ -84,12 +82,12 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. | |||
| - `DISABLE_ROUTER_LOG`: **false**: Mutes printing of the router log. | |||
| - `CERT_FILE`: **custom/https/cert.pem**: Cert file path used for HTTPS. | |||
| - `KEY_FILE`: **custom/https/key.pem**: Key file path used for HTTPS. | |||
| - `STATIC_ROOT_PATH`: **./**Upper level of template and static files path. | |||
| - `STATIC_ROOT_PATH`: **./**: Upper level of template and static files path. | |||
There was a problem hiding this comment.
it would be useful to specify what . is relative to, in this case. Directory containing executable file ? Directory from which the command was run ?
tboerger
added
lgtm/need 1
tboerger
added
lgtm/done
This change adds a default setting to each configuration option present in the cheat sheet. It also cleans up some of the language and grammar.