This patch contains some fixes Jonathan and I made at the CDK workshop in Cambridge. Its mostly tagging classes/methods as thread safe (or not), fixing some thread-related issues, fixing some other issues and also some minor code cleanup.
I see no problems with that.
Patch looks OK, however I think it'd be a good idea to use annotations/tags rather than free text.
So methods that are deemed threadsafe should have a Javadoc tag @threadsafe and those that are not would have @notthreadsafe
This way a taglet can be written to generate a summary report, allowing devs to keep track of what has and has not been checked.
Also I don't see the point of stuff such as
The previous version (-) was pretty concise - why change it to the expanded version? I realize that this is generally a personal preference, but unless it is egregiously unformatted, I don't see a need to change it
I also favor the use of special Javadoc-tags.
Regarding the formatting: that is certainly nothing that needs to be changed. These changes came from the auto-formatting we use in Eclipse for KNIME.
OK, the patch needs updating for the annotation tags introduced in:
instead of lines in the JavaDoc.
I also like to see the use of final explained (not necessarily in the patch, but perhaps as separate patch), what role it has in thread safety... this could potentially lead to a unit test...
I also note JavaDoc stubs like:
which interrupt the JavaDoc quality checking; please either: 1) remove such template JavaDoc, 2) complete that it.
I also second Rajarshi's comment that changes like:
are not necessarily needed, and actually make it much more work to review patches, as I now have to decide for the above if is is just style/layout or a secretly sneaking in code changes... I guess not, but I will have to check that one by one...
Any updates on this?