sergey-chugunov-1985 commented on code in PR #12268:
URL: https://github.com/apache/ignite/pull/12268#discussion_r2285024075
##########
modules/commons/src/main/java/org/apache/ignite/internal/util/CommonUtils.java:
##########
@@ -561,7 +591,7 @@ public static ClassLoader resolveClassLoader(@Nullable
ClassLoader ldr, @Nullabl
}
/**
- * Tests whether given class is loadable by provided class loader.
+ * Tests whether or not given class is loadable provided class loader.
Review Comment:
`or not` is redundant, phase looks better without it. Also `is loadable by`
or `is loadable with` - I suspect without a preposition the phase is incorrect.
Please also go through the whole `CommonUtils` class and fix `whether or
not` form.
##########
modules/commons/src/main/java/org/apache/ignite/internal/util/CommonUtils.java:
##########
@@ -637,7 +667,7 @@ else if (!includePrimitiveTypes || cls.length() > 7 ||
(clazz = primitiveMap.get
public static Class<?> forName(
String clsName,
@Nullable ClassLoader ldr,
- @Nullable IgnitePredicate<String> clsFilter,
Review Comment:
We should restore `Nullable` here and add it in IgniteUtils#forName. Reason:
there are multiple instances in the code where it could be called with a null
reference, BinaryContext#descriptorForTypeId, TcpIgniteClient#getClass to name
a few.
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
