nit: spotbugs spotbugs#186
Conversation
krumts
left a comment
krumts
left a comment
There was a problem hiding this comment.
Thanks for taking the time to tackle the spotbugs warinings!
LGTM overall, I just have one question (inline).
| public final class SpecFactory extends RegistryReader<SpecFactory.Report> | ||
| { | ||
| public class Report | ||
| public static class Report |
There was a problem hiding this comment.
I wonder if this is a breaking API change, WDYT?
org.eclipse.mat.report is one of the exported packages.
There was a problem hiding this comment.
Good observation. I think if one sets the API Baseline in Eclipse and then loads up the new code, it should show if there's an API breakage or not due to this.
kgibm
left a comment
kgibm
left a comment
There was a problem hiding this comment.
Looks good. There was one case where the increment of i++ was moved into the if which changed the calculation slightly (equivalent functionality would be ++i) but it was just a cancel progress check so the exact value doesn't really matter, but it's just a mental note that there are risks of functional changes to this cleanup, so worth keeping that in the front of mind, but overall this is great that you're doing this.
3 participants
No description provided.