You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/06/09 13:21:35 UTC
[GitHub] [commons-jcs] arturobernalg opened a new pull request #71: Simplify conditions avoiding unnecessary validation
arturobernalg opened a new pull request #71:
URL: https://github.com/apache/commons-jcs/pull/71
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jcs] tvand merged pull request #71: Simplify conditions avoiding unnecessary validation
Posted by GitBox <gi...@apache.org>.
tvand merged pull request #71:
URL: https://github.com/apache/commons-jcs/pull/71
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jcs] tvand commented on a change in pull request #71: Simplify conditions avoiding unnecessary validation
Posted by GitBox <gi...@apache.org>.
tvand commented on a change in pull request #71:
URL: https://github.com/apache/commons-jcs/pull/71#discussion_r705425201
##########
File path: commons-jcs-core/src/main/java/org/apache/commons/jcs3/auxiliary/disk/jdbc/mysql/MySQLDiskCacheFactory.java
##########
@@ -102,11 +102,8 @@ protected void scheduleOptimizations( final MySQLDiskCacheAttributes attributes,
try
{
final Date[] dates = ScheduleParser.createDatesForSchedule( attributes.getOptimizationSchedule() );
Review comment:
Would you please consider documenting "never null" as the return value of ScheduleParser.createDatesForSchedule()? The same goes for other calls.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jcs] arturobernalg commented on a change in pull request #71: Simplify conditions avoiding unnecessary validation
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #71:
URL: https://github.com/apache/commons-jcs/pull/71#discussion_r705898459
##########
File path: commons-jcs-jcache/src/main/java/org/apache/commons/jcs3/jcache/JCSCachingManager.java
##########
@@ -189,8 +189,8 @@ private void assertNotClosed()
assertNotClosed();
assertNotNull(cacheName, "cacheName");
assertNotNull(configuration, "configuration");
- final Class<?> keyType = configuration == null ? Object.class : configuration.getKeyType();
Review comment:
Hi @rmannibucau
Can you be more specific. Keep in mind i'm not an expert in JCS and still i´am learning about the logic of the project..
TY
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jcs] tvand merged pull request #71: Simplify conditions avoiding unnecessary validation
Posted by GitBox <gi...@apache.org>.
tvand merged pull request #71:
URL: https://github.com/apache/commons-jcs/pull/71
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jcs] rmannibucau commented on a change in pull request #71: Simplify conditions avoiding unnecessary validation
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #71:
URL: https://github.com/apache/commons-jcs/pull/71#discussion_r705913097
##########
File path: commons-jcs-jcache/src/main/java/org/apache/commons/jcs3/jcache/JCSCachingManager.java
##########
@@ -189,8 +189,8 @@ private void assertNotClosed()
assertNotClosed();
assertNotNull(cacheName, "cacheName");
assertNotNull(configuration, "configuration");
- final Class<?> keyType = configuration == null ? Object.class : configuration.getKeyType();
Review comment:
Expected pattern was `final var foo = a == null ? defaultvalue : a`. Here the default is object class so the configuration == null should be replaced by configuration.getKeyType() or configuration.getValueType() I think.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jcs] arturobernalg commented on a change in pull request #71: Simplify conditions avoiding unnecessary validation
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #71:
URL: https://github.com/apache/commons-jcs/pull/71#discussion_r705898036
##########
File path: commons-jcs-jcache/src/main/java/org/apache/commons/jcs3/jcache/cdi/CacheResolverFactoryImpl.java
##########
@@ -50,7 +50,7 @@ public CacheResolver getCacheResolver(final CacheMethodDetails<? extends Annotat
public CacheResolver getExceptionCacheResolver(final CacheMethodDetails<CacheResult> cacheMethodDetails)
{
final String exceptionCacheName = cacheMethodDetails.getCacheAnnotation().exceptionCacheName();
- if (exceptionCacheName == null || exceptionCacheName.isEmpty())
+ if (exceptionCacheName.isEmpty())
Review comment:
indeed,
TY
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jcs] rmannibucau commented on a change in pull request #71: Simplify conditions avoiding unnecessary validation
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #71:
URL: https://github.com/apache/commons-jcs/pull/71#discussion_r705426326
##########
File path: commons-jcs-jcache/src/main/java/org/apache/commons/jcs3/jcache/JCSCachingManager.java
##########
@@ -189,8 +189,8 @@ private void assertNotClosed()
assertNotClosed();
assertNotNull(cacheName, "cacheName");
assertNotNull(configuration, "configuration");
- final Class<?> keyType = configuration == null ? Object.class : configuration.getKeyType();
Review comment:
didnt check them all but this one was a typo configuration should check configuration.getKeyType() (resp value) to default to Object so instead of dropping it I would fix it, wdyt?
##########
File path: commons-jcs-jcache/src/main/java/org/apache/commons/jcs3/jcache/cdi/CacheResolverFactoryImpl.java
##########
@@ -50,7 +50,7 @@ public CacheResolver getCacheResolver(final CacheMethodDetails<? extends Annotat
public CacheResolver getExceptionCacheResolver(final CacheMethodDetails<CacheResult> cacheMethodDetails)
{
final String exceptionCacheName = cacheMethodDetails.getCacheAnnotation().exceptionCacheName();
- if (exceptionCacheName == null || exceptionCacheName.isEmpty())
+ if (exceptionCacheName.isEmpty())
Review comment:
this is likely a false positive, can be 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jcs] tvand merged pull request #71: Simplify conditions avoiding unnecessary validation
Posted by GitBox <gi...@apache.org>.
tvand merged pull request #71:
URL: https://github.com/apache/commons-jcs/pull/71
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org