jaydoane commented on code in PR #4813:
URL: https://github.com/apache/couchdb/pull/4813#discussion_r1372580930


##########
src/config/src/config.erl:
##########
@@ -252,7 +251,7 @@ init(IniFiles) ->
     ets:new(?MODULE, [named_table, set, protected, {read_concurrency, true}]),
     lists:map(
         fun(IniFile) ->
-            {ok, ParsedIniValues} = parse_ini_file(IniFile),
+            {ok, ParsedIniValues} = parse_ini_file(IniFile, fun delete_keys/1),

Review Comment:
   The only one I'm not entirely sure about is when there's a value in 
`default.ini`, but `local.ini` has a delete marker.  Shouldn't row 3 column 2 
be the same as row 3 column 1, based on this paragraph in  [the 
docs](https://docs.couchdb.org/en/stable/config/intro.html#setting-parameters-via-the-configuration-file)?
   
   > In case when you’d like to remove some parameter from the default.ini 
without modifying that file, you may override in local.ini, but without any 
value
   ```
   [compactions]
   _default =
   ```
   
   But maybe I'm misunderstanding your table and/or the docs.
   
   FWIW, it seems like eunit and elixir tests continue to pass if we replace 
`delete_keys/1` with `omit_delete/1` in the `init` function.
   



##########
src/config/src/config.erl:
##########
@@ -252,7 +251,7 @@ init(IniFiles) ->
     ets:new(?MODULE, [named_table, set, protected, {read_concurrency, true}]),
     lists:map(
         fun(IniFile) ->
-            {ok, ParsedIniValues} = parse_ini_file(IniFile),
+            {ok, ParsedIniValues} = parse_ini_file(IniFile, fun delete_keys/1),

Review Comment:
   The only one I'm not entirely sure about is when there's a value in 
`default.ini`, but `local.ini` has a delete marker.  Shouldn't row 3 column 2 
be the same as row 3 column 1, based on this paragraph in  [the 
docs](https://docs.couchdb.org/en/stable/config/intro.html#setting-parameters-via-the-configuration-file)?
   
   > In case when you’d like to remove some parameter from the default.ini 
without modifying that file, you may override in local.ini, but without any 
value
   ```
   [compactions]
   _default =
   ```
   
   But maybe I'm misunderstanding your table and/or the docs.
   
   FWIW, it seems like eunit and elixir tests continue to pass if we replace 
`delete_keys/1` with `omit_delete/1` in the `init` function.
   



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to