Polish #1938
Polish #1938
Conversation
There was a problem hiding this comment.
That full package name is actually on purpose as the Spring Cache abstraction has also a concept of Cache.
There was a problem hiding this comment.
@snicoll If so, it should be {@link Cache javax.cache.Cache}, not {@link javax.cache.Cache} as the latter will be rendered as just Cache in Javadoc. Is it okay to change to {@link Cache javax.cache.Cache}?
There was a problem hiding this comment.
Oh dear, good catch. Yes please.
There was a problem hiding this comment.
Without the fully qualified package name, we'd want to have "JCache" there in addition.
So we might as well just leave it as is for clarity, as was originally intended.
Thus, please either undo this change or prefix that with "JCache" as is done elsewhere in the Javadoc for this class.
There was a problem hiding this comment.
@sbrannen It looks the same concern as @snicoll 's #1938 (comment). I answered there and let me know what you think.
There was a problem hiding this comment.
The package name starts with an "o". Thus this is grammatically correct.
Please undo this change.
There was a problem hiding this comment.
@sbrannen "a" is intended for the rendered link in Javadoc, "JCacheCache", not "org.springframework.cache.jcache.JCacheCache". Am I missing something?
There was a problem hiding this comment.
Hmmm.... good question!
I believe we (i.e., the collective core Spring Team) were thinking that the fully qualified class name (FQCN) shows up in the generated HTML when linked via FQCN.
So, if that's not the case, that would mean there is a larger issue across the entire code base in the core Spring Framework, since we routinely use "an" instead of "a" in such cases.
Perhaps it's better to address that nuanced feature of Javadoc in a dedicated JIRA issue.
@snicoll, thoughts?
There was a problem hiding this comment.
@sbrannen thanks. Could you please raise a SPR issue for this? I got caught as well and I am wondering if that's not a new feature of javadoc or something (or I've been wrong this all time).
There was a problem hiding this comment.
Indeed! I am also wondering if this was a change to how Javadoc works in a recent release (perhaps in JDK 8?).
And well... if you've been wrong this whole time, so have I and many others. 😉
I'll raise a JIRA issue to track this.
There was a problem hiding this comment.
There was a problem hiding this comment.
The package name starts with an "o". Thus this is grammatically correct.
Please undo this change.
This comment has been minimized.
This comment has been minimized.
sbrannen
dismissed
their stale review
@snicoll had reviewed this PR simultaneously.
|
@snicoll, after our combined reviews, it appears that the only remaining polishing would be the removal of the extra space in the exception message. Thus, I've dismissed my review and will let you take it from here. Cheers, Sam |
|
Thanks a lot @izeye, this is now merged in |
This PR fixes some typos and polishes trivial stuff.