You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/06/21 13:31:40 UTC

[GitHub] [couchdb] jiahuili430 opened a new pull request #3636: Normalize config options main

jiahuili430 opened a new pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636


   ## Overview
   Normalize some configuration options to comment out the default.ini file.
   Solved conflicts from "cherry-pick" 3.x commit with kdiff3 merge tool.
   
   ## Testing recommendations
   
   
   ## Related Issues or Pull Requests
   fixes #3473
   
   ## Checklist
   
   - [x] Code is written and works correctly
   - [ ] Changes are covered by tests
   - [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
   - [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636#discussion_r657317387



##########
File path: src/chttpd/src/chttpd_prefer_header.erl
##########
@@ -47,8 +52,7 @@ filter_headers(Headers, IncludeList) ->
 
 
 get_header_list() ->
-    SectionStr = config:get("chttpd", "prefer_minimal", ""),
-    split_list(SectionStr).
+    split_list(config:get("chttpd", "prefer_minimal", ?DEFAULT_PREFER_MINIMA)).

Review comment:
       typo




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636#discussion_r657315445



##########
File path: src/chttpd/src/chttpd_prefer_header.erl
##########
@@ -22,6 +22,11 @@
 -include_lib("couch/include/couch_db.hrl").
 
 
+-define(DEFAULT_PREFER_MINIMA,

Review comment:
       Typo. It should be `DEFAULT_PREFER_MINIMAL`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636#discussion_r657313527



##########
File path: src/chttpd/src/chttpd.erl
##########
@@ -148,7 +151,12 @@ start_link(Name, Options) ->
     end.
 
 get_server_options(Module) ->
-    ServerOptsCfg = config:get(Module, "server_options", "[]"),
+    ServerOptsCfg =
+        case Module of
+            "chttpd" -> config:get(Module,

Review comment:
       strange indentation (we use `clause -> do_something` only if each clause fit on a single line).
   ```
   case Module of
      "chttpd" ->
           config:get(Module, "server_options", ?DEFAULT_SERVER_OPTIONS);
       _ ->
           config:get(Module, "server_options", "[]")
   end,
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] jiahuili430 commented on a change in pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
jiahuili430 commented on a change in pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636#discussion_r657318226



##########
File path: src/chttpd/src/chttpd_prefer_header.erl
##########
@@ -47,8 +52,7 @@ filter_headers(Headers, IncludeList) ->
 
 
 get_header_list() ->
-    SectionStr = config:get("chttpd", "prefer_minimal", ""),
-    split_list(SectionStr).
+    split_list(config:get("chttpd", "prefer_minimal", ?DEFAULT_PREFER_MINIMA)).

Review comment:
       Changed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] jiahuili430 commented on a change in pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
jiahuili430 commented on a change in pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636#discussion_r657315714



##########
File path: src/chttpd/src/chttpd.erl
##########
@@ -148,7 +151,12 @@ start_link(Name, Options) ->
     end.
 
 get_server_options(Module) ->
-    ServerOptsCfg = config:get(Module, "server_options", "[]"),
+    ServerOptsCfg =
+        case Module of
+            "chttpd" -> config:get(Module,

Review comment:
       Changed.
   Thanks for the review, @iilyak 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak merged pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
iilyak merged pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] jiahuili430 commented on a change in pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
jiahuili430 commented on a change in pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636#discussion_r657318064



##########
File path: src/chttpd/src/chttpd_prefer_header.erl
##########
@@ -22,6 +22,11 @@
 -include_lib("couch/include/couch_db.hrl").
 
 
+-define(DEFAULT_PREFER_MINIMA,

Review comment:
       Changed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
iilyak commented on pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636#issuecomment-867531612


    Very nice work. Thank you.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] jiahuili430 commented on a change in pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
jiahuili430 commented on a change in pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636#discussion_r657372608



##########
File path: src/chttpd/test/eunit/chttpd_util_test.erl
##########
@@ -19,24 +19,24 @@
 
 setup() ->
     ok = config:set("httpd", "both_exist", "get_in_httpd", _Persist = false),
-    ok = config:set("chttpd", "both_exist", "get_in_chttpd", _Persist = false),
-    ok = config:set("httpd", "httpd_only", "true", _Persist = false),
-    ok = config:set("chttpd", "chttpd_only", "1", _Persist = false),
-    ok = config:set("couch_httpd_auth", "both_exist", "cha", _Persist = false),
-    ok = config:set("chttpd_auth", "both_exist", "ca", _Persist = false),
-    ok = config:set("couch_httpd_auth", "cha_only", "true", _Persist = false),
-    ok = config:set("chttpd_auth", "ca_only", "1", _Persist = false).
+    ok = config:set("chttpd", "both_exist", "get_in_chttpd", false),

Review comment:
       Changed back.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] iilyak commented on a change in pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636#discussion_r657319071



##########
File path: src/chttpd/test/eunit/chttpd_util_test.erl
##########
@@ -19,24 +19,24 @@
 
 setup() ->
     ok = config:set("httpd", "both_exist", "get_in_httpd", _Persist = false),
-    ok = config:set("chttpd", "both_exist", "get_in_chttpd", _Persist = false),
-    ok = config:set("httpd", "httpd_only", "true", _Persist = false),
-    ok = config:set("chttpd", "chttpd_only", "1", _Persist = false),
-    ok = config:set("couch_httpd_auth", "both_exist", "cha", _Persist = false),
-    ok = config:set("chttpd_auth", "both_exist", "ca", _Persist = false),
-    ok = config:set("couch_httpd_auth", "cha_only", "true", _Persist = false),
-    ok = config:set("chttpd_auth", "ca_only", "1", _Persist = false).
+    ok = config:set("chttpd", "both_exist", "get_in_chttpd", false),

Review comment:
       Why remove `_Persist = `?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [couchdb] jiahuili430 commented on a change in pull request #3636: Normalize config options main

Posted by GitBox <gi...@apache.org>.
jiahuili430 commented on a change in pull request #3636:
URL: https://github.com/apache/couchdb/pull/3636#discussion_r657321303



##########
File path: src/chttpd/test/eunit/chttpd_util_test.erl
##########
@@ -19,24 +19,24 @@
 
 setup() ->
     ok = config:set("httpd", "both_exist", "get_in_httpd", _Persist = false),
-    ok = config:set("chttpd", "both_exist", "get_in_chttpd", _Persist = false),
-    ok = config:set("httpd", "httpd_only", "true", _Persist = false),
-    ok = config:set("chttpd", "chttpd_only", "1", _Persist = false),
-    ok = config:set("couch_httpd_auth", "both_exist", "cha", _Persist = false),
-    ok = config:set("chttpd_auth", "both_exist", "ca", _Persist = false),
-    ok = config:set("couch_httpd_auth", "cha_only", "true", _Persist = false),
-    ok = config:set("chttpd_auth", "ca_only", "1", _Persist = false).
+    ok = config:set("chttpd", "both_exist", "get_in_chttpd", false),

Review comment:
       I ran `emilio` before and it gave a warning about `_Persist`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org