jonmeredith commented on code in PR #1758:
URL: https://github.com/apache/cassandra/pull/1758#discussion_r932618899


##########
src/java/org/apache/cassandra/config/YamlConfigurationLoader.java:
##########
@@ -363,10 +365,12 @@ else if (result.getAnnotation(Deprecated.class) != null)
 
             return new ForwardingProperty(result.getName(), result)
             {
+                boolean allowsNull = result.getAnnotation(Nullable.class) != 
null;
+
                 @Override
                 public void set(Object object, Object value) throws Exception
                 {
-                    if (value == null && get(object) != null)
+                    if (value == null && get(object) != null && !allowsNull)

Review Comment:
   It seems inconsistent to only require an `Nullable` annotation for 
configuration that can be set null, but is not defaulted to be null.  What do 
you think about dropping the `get(object) != null` check, and applying 
`@Nullable` to all cases in Config that can be legitimately set to `null`? 



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