[GitHub] [couchdb] jaydoane commented on a diff in pull request #4193: A few more cleanups in fabric_doc_open_revs

[GitBox <gi...@apache.org>]

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


##########
src/fabric/src/fabric_doc_open_revs.erl:
##########
@@ -139,20 +123,40 @@ handle_message({ok, RawReplies}, Worker, State) ->
                 IsTree,
                 NewReplies,
                 NewNodeRevs,
-                ReplyCount + 1,
+                NewReplyCount,
                 InRepair orelse Repair
             ),
             {stop, format_reply(IsTree, NewReplies, RealReplyCount)};
         false ->
             {ok, State#state{
                 replies = NewReplies,
                 node_revs = NewNodeRevs,
-                reply_count = ReplyCount + 1,
+                reply_count = NewReplyCount,
                 workers = lists:delete(Worker, Workers),
                 repair = InRepair orelse Repair
             }}
     end.
 
+% Update node revs from the latest replies
+%
+update_node_revs(nil, _, PrevNodeRevs) ->
+    % This handles the error cases when Worker is explicitly set to nil (see
+    % rexi_DOWN and rexi_EXIT clauses

Review Comment:
   append ")." to close parenthetical statement



##########
src/fabric/src/fabric_doc_open_revs.erl:
##########
@@ -139,20 +123,40 @@ handle_message({ok, RawReplies}, Worker, State) ->
                 IsTree,
                 NewReplies,
                 NewNodeRevs,
-                ReplyCount + 1,
+                NewReplyCount,
                 InRepair orelse Repair
             ),
             {stop, format_reply(IsTree, NewReplies, RealReplyCount)};
         false ->
             {ok, State#state{
                 replies = NewReplies,
                 node_revs = NewNodeRevs,
-                reply_count = ReplyCount + 1,
+                reply_count = NewReplyCount,
                 workers = lists:delete(Worker, Workers),
                 repair = InRepair orelse Repair
             }}
     end.
 
+% Update node revs from the latest replies
+%
+update_node_revs(nil, _, PrevNodeRevs) ->
+    % This handles the error cases when Worker is explicitly set to nil (see
+    % rexi_DOWN and rexi_EXIT clauses
+    PrevNodeRevs;
+update_node_revs(Worker, RawReplies, PrevNodeRevs) ->
+    FoldFun = fun
+        ({ok, #doc{revs = {Pos, [Rev | _]}}}, Acc) ->
+            [{Pos, Rev} | Acc];
+        (_, Acc) ->
+            Acc
+    end,
+    case lists:foldl(FoldFun, [], RawReplies) of
+        [] ->
+            PrevNodeRevs;
+        [_ | _] = IdRevs ->
+            [{Worker#shard.node, IdRevs} | PrevNodeRevs]
+    end.

Review Comment:
   This is a great readability improvement over the previous logic 👏 



-- 
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: notifications-unsubscribe@couchdb.apache.org

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