Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 338960 - Diagram Editor needs to log error messages instead of using System.err and printStackTrace
Summary: Diagram Editor needs to log error messages instead of using System.err and pr...
Status: RESOLVED FIXED
Alias: None
Product: Dali JPA Tools
Classification: WebTools
Component: Diagram Editor (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.0 RC1   Edit
Assignee: Bistra Yakimova CLA
QA Contact: Stefan Dimov CLA
URL:
Whiteboard: PMC_approved
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-04 11:54 EST by Karen Butzke CLA
Modified: 2011-05-13 13:25 EDT (History)
4 users (show)

See Also:
stefan.dimov: pmc_approved? (david_williams)
stefan.dimov: pmc_approved? (raghunathan.srinivasan)
stefan.dimov: pmc_approved? (naci.dai)
stefan.dimov: pmc_approved? (deboer)
stefan.dimov: pmc_approved? (neil.hauge)
stefan.dimov: pmc_approved? (kaloyan)
cbridgha: pmc_approved+
stefan.dimov: review+
neil.hauge: review+

Attachments
Patch with log error messages instead of System.err and printStackTrace (5.26 KB, patch)
2011-05-04 11:14 EDT, Bistra Yakimova CLA 		no flags Details | Diff
Patch proposal (51.99 KB, patch)
2011-05-11 03:24 EDT, Bistra Yakimova CLA 		no flags Details | Diff
exception logging improved (50.64 KB, patch)
2011-05-12 05:07 EDT, Stefan Dimov CLA 		no flags Details | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Karen Butzke CLA 2011-03-04 11:54:31 EST 
The diagram editor currently catches exceptions and prints an error using System.err and printStackTrace(). It instead needs to be logging the exception in the JPADiagramEditorPlugin. You can copy the logging code from the JptJpaCorePlugin, there are for static log methods that are useful
Comment 1 Bistra Yakimova CLA 2011-05-04 11:14:17 EDT 
Created attachment 194725 [details]
Patch with log error messages instead of System.err and printStackTrace
Comment 2 Bistra Yakimova CLA 2011-05-11 03:24:43 EDT 
Created attachment 195302 [details]
Patch proposal
Comment 3 Stefan Dimov CLA 2011-05-11 11:28:49 EDT 
Neil, do you think it's worth to get in Indigo? The change is relatively big, but it concerns only exception handling.
Comment 4 Neil Hauge CLA 2011-05-11 11:50:15 EDT 
I think this is worth trying to get in.
Comment 5 Stefan Dimov CLA 2011-05-11 11:53:08 EDT 
Yeah, me too. I'll have it tested 'till tomorrow ...
Comment 6 Stefan Dimov CLA 2011-05-12 05:07:10 EDT 
Created attachment 195477 [details]
exception logging improved

The first patch is fine, but there was a small incompatibility - result of the previous submit (of another patch), so I'm submitting a second version
Comment 7 Stefan Dimov CLA 2011-05-12 05:22:20 EDT 
Every Eclipse plugin is supposed to be aligned with the Eclipse standards and guidelines. In this particular case this means that the diagram editor should log errors and exceptions just like any other plugin. 

I've performed a manual sanity check to see if the diagram could be open, created, manipulated, changed and closed. All the existing JUnit tests are passing successfully. 

Also, I've reviewed every single line of the patch visually in order to check that it affects logging only. There were a few new logging methods added in JPADiagramEditorPlugin. All the other changes affect the logging - calling the new methods in JPADiagramEditorPlugin instead of printing the stack trace and no existing functionality was changed.

The risk is low.
Comment 8 Chuck Bridgham CLA 2011-05-12 09:39:31 EDT 
Changes are good - thanks
Comment 9 Tran Le CLA 2011-05-12 11:19:54 EDT 
Patch committed for rc1