<Swing Dev> Review Request for 8039966: fix doclint block tag issues in swing filechooser and colorchooser classes
sergey malenkov
sergey.malenkov at oracle.com
Wed Apr 30 09:09:04 UTC 2014
Now it looks OK for CCC request:
http://ccc.us.oracle.com/docs/process
Thanks,
SAM
On 30.04.2014 2:56, Steve Sides wrote:
> Review Request for 8039966: fix doclint block tag issues in swing
> filechooser and colorchooser classes
>
> A few more changes, mostly just as noted.
> http://cr.openjdk.java.net/~ssides/8039966/8039966.5
>
> -steve
>
> On 4/25/2014 8:34 AM, sergey malenkov wrote:
>> Hi Steve,
>>
>> FileSystemView:
>>
>> /**
>> * Returns a File object constructed from the given path string.
>> + *
>> + * @param path {@code String} representation of path
>> + * @return a {@code File} object created from the given String
>> */
>> public File createFileObject(String path) {
>>
>>
>> I think, that "from the given {@code path}" is better than "from the
>> given String".
>>
>> /**
>> * Gets the list of shown (i.e. not hidden) files.
>> + *
>> + * @param dir the root directory of files to be returned
>> + * @param useFileHiding determine if hidden files are returned
>> + * @return an array of {@code File} objects. Include hidden
>> files if
>> + * {@code useFileHiding} is false.
>> */
>>
>> I suggest to rewrite last sentence:
>> @ return an array of {@code File} objects representing files and
>> directories in the given {@code dir}.
>> It includes hidden files if {@code useFileHiding} is {@code false}.
>>
>> FileView:
>>
>> Note, that all @return tags differ from each other.
>> + * @return a String containing a description of the file
>> + * @return a {@code String} containing a description of the type
>> of the file
>> + * @return an {@code Icon} which represents the specified {@code
>> File}
>> I recommend to use something like this:
>> @return a {@code String} containing a description of the given file,
>> or {@code null} if it is not available.
>>
>> /**
>> * Whether the directory is traversable or not. This might be
>> * useful, for example, if you want a directory to represent
>> * a compound document and don't want the user to descend into it.
>> + *
>> + * @param f a {@code File} object representing a directory
>> + * @return true if the directory is traversable
>> */
>> public Boolean isTraversable(File f) {
>>
>> Note that the method may return null. We should describe it somehow.
>> For example,
>> @return true if the directory is traversable,
>> false if it is not traversable,
>> null if the file system should be asked.
>> @see FileSystemView#isTraversable
>>
>> Thanks,
>> SAM
>>
>>
>>
>> On 24.04.2014 22:00, Steve Sides wrote:
>>> Hi Sergey, et. all,
>>> changes as per comments:
>>> http://sqeweb.us.oracle.com/coretools/ssides/webrevs/8039966/8039966.5/
>>>
>>> Some comments in line...
>>>
>>> On 4/24/2014 5:16 AM, sergey malenkov wrote:
>>>> Hello,
>>>>
>>>> /**
>>>> * The icon that represents this file in the
>>>> <code>JFileChooser</code>.
>>>> + *
>>>> + * @param f a {@code File} object
>>>> + * @return an {@code Icon} to be displayed by the file chooser
>>>> */
>>>> public Icon getIcon(File f) {
>>>>
>>>> What if the FileView class is used from another component, not the
>>>> file chooser.
>>>> I prefer something like this: an {@code Icon} that represent the
>>>> specified {@code File}.
>>> I see your point, although, this is in the 'filechooser' package and
>>> is documented as
>>> "an abstract class that can be implemented to provide the
>>> filechooser with UI information for a |File|."
>>> However, I used a return comment more akin to what you suggested.
>>>
>>>>
>>>>
>>>> /**
>>>> * Returns all root partitions on this system. For example, on
>>>> * Windows, this would be the "Desktop" folder, while on DOS this
>>>> * would be the A: through Z: drives.
>>>> + *
>>>> + * @return an array of {@code File} objects
>>>> */
>>>> public File[] getRoots() {
>>>>
>>>> It is obvious that this method returns an array of File objects.
>>>> It is better to specify these objects. For example:
>>>>
>>>> + * @return an array of File objects which represent root
>>>> partitions of this file system.
>>> got it.
>>>>
>>>> /**
>>>> * Returns a File object constructed in dir from the given
>>>> filename.
>>>> + * If {@code dir} is {@code null}, then the new {@code File}
>>>> instance is
>>>> + * created as if by invoking the single-argument {@code File}
>>>> constructor
>>>> + * on the given {@code filename} string.
>>>> + *
>>>> + * @param dir an abstract pathname denoting a directory
>>>> + * @param filename a {@code String} representation of a pathname
>>>> + * @return a {@code File} object created from {@code dir} and
>>>> {@code filename}
>>>> */
>>>> public File createFileObject(File dir, String filename) {
>>>>
>>>> Seems this javadoc describes an implementation details.
>>>> We should specify here that the created File object represents a
>>>> file with specified filename, that is located in specified folder.
>>> I was looking into what this does and meandered on to the File doc
>>> and kind of borrowed the javadoc idea from there, since this
>>> does the same. I put it back to the original.
>>>
>>> -steve
>>>
>>>>
>>>> etc...
>>>>
>>>>
>>>> Note that all such changes in the API require an approval from CCC.
>>>> This is not a small typo. You really change the *public* API!
>>>>
>>>> Thanks
>>>> SAM
>>>>
>>>> On 18.04.2014 2:57, Steve Sides wrote:
>>>>> Hello,
>>>>>
>>>>> Could you please review the fix for the following bug:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8039966
>>>>>
>>>>> Webrev corresponding:
>>>>> http://cr.openjdk.java.net/~ssides/8039966/8039966.4/
>>>>>
>>>>> This addresses missing @parm and @return block tags in javadoc for
>>>>> javax/swing/colorchooser and filechooser classes as noted by doclint.
>>>>>
>>>>> It does not address methods which are missing javadoc comments
>>>>> completely.
>>>>>
>>>>> You may want to check the wording of @param for
>>>>> (un)installChooserPanel(). The original seemed a little incorrect
>>>>> to me.
>>>>>
>>>>> thanks,
>>>>>
>>>>> -steve
>>>>>
>>>>
>>>
>>
>
More information about the swing-dev
mailing list