iilyak commented on a change in pull request #3766:
URL: https://github.com/apache/couchdb/pull/3766#discussion_r731726910
##########
File path: src/smoosh/src/smoosh_priority_queue.erl
##########
@@ -73,14 +113,53 @@ info(#priority_queue{tree=Tree}=Q) ->
[{min, Min}, {max, Max}]
end].
+from_list(Orddict, #priority_queue{name=Name}) ->
+ Map = maps:from_list(Orddict),
+ Tree = gb_trees:from_orddict(Orddict),
+ #priority_queue{name=Name, map=Map, tree=Tree}.
+
+to_list(#priority_queue{tree=Tree}) ->
+ gb_trees:to_list(Tree).
+
+file_name(#priority_queue{name=Name}) ->
+ filename:join(".", Name ++ ".queue").
Review comment:
This doesn't seem right. The `filename:join/2` is designed to create a
path. Production deployments of CouchDB might store data on separate filesystem
which means that the directory need to be configurable.
You probably want something like:
```
filename:join(config:get("smoosh", "state_dir", "."), Name ++ ".queue").
```
However this means that the module gains knowledge about `config` module.
Which makes it harder to test. The alternative is to pass directory to `new`.
I don't have strong preference.
--
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]
