aweisberg commented on code in PR #7:
URL: https://github.com/apache/cassandra-accord/pull/7#discussion_r1014312544


##########
accord-core/src/main/java/accord/messages/PreAccept.java:
##########
@@ -121,7 +142,7 @@ default MessageType type()
             return MessageType.PREACCEPT_RSP;
         }
 
-        boolean isOK();
+        boolean isOk();
     }
 
     public static class PreAcceptOk implements PreAcceptReply

Review Comment:
   I don't understand why `PartialDeps` would be confusing. `Deps` is just 
`PartialDeps` for all ranges and both `Deps` and `PartialDeps` can be 
incomplete in terms of containing all the dependencies since they must be 
collected.
   
   What would be nice is if `PartialDeps` and `Deps` were subclasses of 
`AbstractDeps`, sure a `PartialDeps` can be converted to a `Deps` through a 
checked conversion in some cases, but that is pretty specialized and it is 
obvious when this conversion is occurring.
   
   In a lot of cases we know we are working with `PartialDeps` or `Deps`?
   
   Maybe the distinction isn't very valuable, but the ambiguity of `Deps` vs 
`PartialDeps` might be worth eliminating.
   
   Don't feel super strongly about this, but in the early stages of onboarding 
when you don't understand the implementation and protocol it's not always so 
obvious what kind of deps you are looking at.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to