Community
Participate
Working Groups
The instruction given in the section doesn't seem to match the behavior described. The error message given should be about the missing corresponding case and not about the default case. Or that's what I see in the IDE when I set the Error/Warnings options as specified. Also note that both the compiler options and warning messages are incorrect at some places in the N&N doc. To be precise, it's "cases over enum" vs "cases on enum".
Stephan, please follow up. Build schedule available here: http://www.eclipse.org/eclipse/platform-releng/buildSchedule.html
Are we talking about this document: http://git.eclipse.org/c/platform/eclipse.platform.common.git/tree/bundles/org.eclipse.jdt.doc.user/whatsNew/jdt_whatsnew.html ? Please note that if screenshots need updating, I need help from s.o. running on the windows platform that was set as the standard. I can only provide KDE/GTK screenshots :)
(In reply to comment #2) > Are we talking about this document: > http://git.eclipse.org/c/platform/eclipse.platform.common.git/tree/bundles/org.eclipse.jdt.doc.user/whatsNew/jdt_whatsnew.html > ? Yes. > Please note that if screenshots need updating, I need help from s.o. running on > the windows platform that was set as the standard. I can only provide KDE/GTK > screenshots :) I can do that. But can you please confirm that the documentation indeed needs correction? The text part of the doc requires a minor change too.
(In reply to comment #3) > I can do that. But can you please confirm that the documentation indeed needs > correction? The text part of the doc requires a minor change too. I think the problem originates from the two-step process of improving these diagnostics. It's probably best to merge the two sections into one. I'll provide a patch for that.
(In reply to comment #4) > It's probably best to merge the two sections into one. That's true for other items in N&N as well e.g. resource leak. CCing Markus as well, since I think he is/was working on the N&N doc.
Created attachment 216812 [details] Screenshot Stephan, can you please check is this screenshot is okay? Markus, I tried to follow the Doc Style guide. But used a slightly lower resolution than recommended (1024 x 768), since my machine has a different ration. Please let me know if this okay. When Stephan confirms, this image with another minor text correction should go into the doc. Just FYI.
(In reply to comment #6) > Created attachment 216812 [details] > Screenshot I attached it anyway since I already had it. Stephan, feel free to ignore it if it is not required with the re-arrangement.
Created attachment 216815 [details] combined section regarding switch (In reply to comment #4) > I think the problem originates from the two-step process of improving these > diagnostics. It's probably best to merge the two sections into one. > I'll provide a patch for that. Here's a version that combines the two sections regarding switch statements. The new switch-case-enum.png actually fits well (provided it's suitable cropped). I think the last image switch-missing-default.png would drive its point home even better, if it contained cases for all three colors from the previous examples. Do you still see inconsistencies? Jay, do you want to commit this after a final touch on the screenshot(s)?
(In reply to comment #5) > (In reply to comment #4) > > It's probably best to merge the two sections into one. > That's true for other items in N&N as well e.g. resource leak. CCing Markus as > well, since I think he is/was working on the N&N doc. At a quick glance I didn't see any strict inconsistencies, so the easiest fix for this might be to just put the paragraphs into order: - New options to detect resource leaks - Resource leak detection improved and enabled by default - Smarter resource leak detection How about a link to ../tasks/task-avoiding_resource_leaks.htm ? I already added a similar link to the switch paragraph (patch in comment 8)
(In reply to comment #9) > At a quick glance I didn't see any strict inconsistencies, so the easiest > fix for this might be to just put the paragraphs into order: > - New options to detect resource leaks > - Resource leak detection improved and enabled by default > - Smarter resource leak detection Inconsistencies is not the only thing. There should really be 1 entry for 1 item i.e. a user does not need to know that we made something 'smarter' after the initial implementation. > How about a link to ../tasks/task-avoiding_resource_leaks.htm ? > I already added a similar link to the switch paragraph (patch in comment 8) Works for me. Also I think you can release any changes you have to master. Markus is not working on JDT N&N right now.
Created attachment 216834 [details] Screenshots I recreated all three screen-shots of code examples. Can you quickly glance them and see if they are fine? I will release the changes in a bit.
(In reply to comment #11) > Created attachment 216834 [details] > Screenshots > > I recreated all three screen-shots of code examples. Can you quickly glance > them and see if they are fine? I will release the changes in a bit. Screenshots look good to me, thanks! (In reply to comment #10) > Inconsistencies is not the only thing. There should really be 1 entry for 1 > item i.e. a user does not need to know that we made something 'smarter' after > the initial implementation. > ... > Also I think you can release any changes you have to master. Markus is not > working on JDT N&N right now. OK, I'll re-arrange the section on resource leaks now.
(In reply to comment #12) > OK, I'll re-arrange the section on resource leaks now. I guess I will wait for you to make those changes as well. Can you combine it with the previous patch if that's not a problem?
Created attachment 216837 [details] switch and resource leak sections combined This patch combines the changes for switch and resource leak sections. I also added a link to background information in the null analysis section, but: it seems that part needs some merging too: I'm planing to combine these paragraphs: - Annotation-based null analysis - Improved messages for null analysis problems
The screenshot null-prefs.png needs an update, please.
Also the following screenshots are out-of-date: - null-annotation-problems1.png old error message and also missing: link to quick fix - null-annotation-problems2.png error message is OK, but quick fixes are missing
Created attachment 216845 [details] combined changes: switch, resource leak, null annotations This patch combines all my text changes regarding incomplete switch, resource leaks and null annotations. Please let me know if you see more work for me.
(In reply to comment #17) > Created attachment 216845 [details] [diff] > combined changes: switch, resource leak, null annotations > > This patch combines all my text changes regarding incomplete switch, resource > leaks and null annotations. I took a very quick look and it looks OK to me. Stephan, I think you can release the patch. Jay, I guess you will take care of the screenshots? > Please let me know if you see more work for me. I am just wondering if some of the screenshots for resource-leak, null-annotations, enum-switch should also find their way to the 'Improving Java code quality' pages?
(In reply to comment #18) > (In reply to comment #17) > > Created attachment 216845 [details] [diff] > > combined changes: switch, resource leak, null annotations > > > > This patch combines all my text changes regarding incomplete switch, resource > > leaks and null annotations. > I took a very quick look and it looks OK to me. Stephan, I think you can > release the patch. Released via commit c4ba4dc673da57960b1c70b2c271d964454e6e9e > I am just wondering if some of the screenshots for resource-leak, > null-annotations, enum-switch should also find their way to the 'Improving Java > code quality' pages? I don't think that many of the exact same screenshots would fit. We might (some day) think of replacing code listings with new screenshots, but with the different focus (education about some backgrounds rather than eye-catching) I don't think screenshots are essential there. Actually, I'd probably prefer pretty printed text, because then users can still copy-n-paste those snippets into an editor ...
Jay, the screenshots have some issues - All 3 switch*.png are too wide. The max width needs to be 519, so that a horizontal scroll bar does not appear in a default sized help window - null-annotation-problems1.png - comment in the second line of bar(..) needs to be 'cannot assign null value' - null-annotation-problems2.png - there is a typo 'methos'
(In reply to comment #15) > The screenshot null-prefs.png needs an update, please. Jay, this is also remaining.
(In reply to comment #20) > Jay, the screenshots have some issues I have adjusted the width so that the scrollbars don't appear. Also fixed the other errors. The screenshots have already been pushed to master and integration.
(In reply to comment #22) > (In reply to comment #20) > > Jay, the screenshots have some issues > > I have adjusted the width so that the scrollbars don't appear. Also fixed the > other errors. The screenshots have already been pushed to master and > integration. Thanks, Jay!
Jay, in general a screenshot should not include the blue line highlight, unless of course you really want to highlight something :-) Though, I think we can let the screenshots be at this point. I also released a couple of minor edits to 2 screenshots.
(In reply to comment #24) > Jay, in general a screenshot should not include the blue line highlight, unless > of course you really want to highlight something :-) Though, I think we can let > the screenshots be at this point. Thanks, I will keep that in mind. I noticed immediately after releasing them last night. But then it was very late and I didn't want to go through another iteration. The first batch of screenshots didn't have them, I think :( Btw, any idea why I am not seeing the changes in I20120605-1900.
(In reply to comment #25) > Btw, any idea why I am not seeing the changes in I20120605-1900. That's the 4.2 build, and as I mentioned yesterday there is a separate branch for that R4_HEAD which is merged with master from time to time. The last merge happened sometime yesterday evening.
