Modify

Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#13318 closed enhancement (fixed)

Clean up program startup (parameters, logging)

Reported by: michael2402 Owned by: team
Priority: normal Milestone: 16.08
Component: Core Version:
Keywords: gsoc-core Cc: Don-vip, bastiK, stoecker

Description

I cleaned up the MainApplication class more.

Things included:

  • A new logging class with log level constants
  • A new class to parse the command line arguments
  • Some cleanup in the initialization tasks

I wrote a new class to log the warning/error messages. The class basically does what main did. But it also traces the source of the log call (as good as the VM allows us to). main is relaying all log calls. So currently you only get messages like (format can be changed):

Aug 11, 2016 3:15:04 PM org.openstreetmap.josm.Main info
INFORMATION: Erweiterung 'ImportImagePlugin' wird geladen (Version 32584)

We have two solutions here:

  1. Ignore the calls from main, use the method that called Main.warn
  2. Deprecate all Main logging methods. This will remove over 200 lines from that class.

I prefer option 2, but it will require a lot of changes to existing source code (but most calls will be search + replace). Stats:

Main.error  325
Main.warn   353
Main.info   109
Main.debug  175
Main.trace  147

There are currently 624 files depending on Main for various reasons. We may reduce that dependency by approx 100 files with this.

Attachments (6)

patch-main-extract-argument-handling.patch (58.6 KB) - added by michael2402 3 years ago.
patch-fix-13318.patch (58.6 KB) - added by michael2402 3 years ago.
patch-fix-13318-2.patch (71.3 KB) - added by michael2402 3 years ago.
13318_patch_pb.PNG (19.6 KB) - added by Don-vip 3 years ago.
patch-fix-13318-4.patch (71.6 KB) - added by michael2402 3 years ago.
patch-fix-13318-tests.patch (2.1 KB) - added by michael2402 3 years ago.

Download all attachments as: .zip

Change History (25)

Changed 3 years ago by michael2402

comment:1 Changed 3 years ago by Don-vip

Patch does not apply cleanly

Changed 3 years ago by michael2402

Attachment: patch-fix-13318.patch added

comment:2 Changed 3 years ago by michael2402

Thanks for the hint. I added a new version ;-)

comment:3 Changed 3 years ago by Don-vip

Still errors with JOSMFixture, and MainTest needs update :)

Changed 3 years ago by michael2402

Attachment: patch-fix-13318-2.patch added

comment:4 Changed 3 years ago by michael2402

Thanks for the hint ;-).

I added a new test for the Logging class, 99% coverage (with result checking)

Changed 3 years ago by Don-vip

Attachment: 13318_patch_pb.PNG added

comment:5 Changed 3 years ago by Don-vip

Still not ok, is there a problem with your branch?


comment:6 Changed 3 years ago by michael2402

It may have been that the git mirror lacks behind master. I'll have a look at it tomorrow.

comment:7 Changed 3 years ago by Don-vip

It's a huge patch to merge manually, can you please help me to get a clean patch?

comment:8 Changed 3 years ago by michael2402

Oh, sorry. I already cleaned it but forgot to attach the patch after travis was through with it.

The newest version is at:
https://github.com/michaelzangl/josm/commit/6eaf0f2bcaf31eaec484228d4989b04645193892

It is 5 days old, so there may already be some comflicts. I'll update the patch to the current JOSM version and then attach it.

Changed 3 years ago by michael2402

Attachment: patch-fix-13318-4.patch added

comment:9 Changed 3 years ago by Don-vip

Resolution: fixed
Status: newclosed

In 10899/josm:

fix #13318 - Clean up program startup (parameters, logging) - patch by michael2402 - gsoc-core

comment:10 Changed 3 years ago by Don-vip

Resolution: fixed
Status: closedreopened
Summary: [Patch] Clean up program startup (parameters, logging)Clean up program startup (parameters, logging)

3 unit test failures:

org.openstreetmap.josm.tools.bugreport.BugReportTest.testGetCallingMethod
org.openstreetmap.josm.tools.bugreport.BugReportTest.testIntercept
org.openstreetmap.josm.tools.bugreport.BugReportTest.testReportText

comment:11 Changed 3 years ago by michael2402

Summary: Clean up program startup (parameters, logging)[Patch] Clean up program startup (parameters, logging)

Fix getCallingMethod offset - there is one more call in between now.

diff, patch, https://travis-ci.org/michaelzangl/josm.svg

Commits:

  1. 0dfb4e2: Fix getCallingMethod offset - there is one more call in between now.

Stats:

 src/org/openstreetmap/josm/tools/bugreport/BugReport.java | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Changed 3 years ago by michael2402

Attachment: patch-fix-13318-tests.patch added

comment:12 Changed 3 years ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 10902/josm:

fix #13318 - fix bug report (patch by michael2402) - gsoc-core

comment:13 Changed 3 years ago by michael2402

In 10909/josm:

See #13318. MainApplication: Make log message for log level display the level.

comment:14 Changed 3 years ago by simon04

Resolution: fixed
Status: closedreopened

r10899 causes this error-prone warning:

    [javac] /home/simon/src/josm.svngit/src/org/openstreetmap/josm/tools/Logging.java:69: warning: [UnsynchronizedOverridesSynchronized] Unsynchronized method publish overrides synchronized method in StreamHandler
    [javac]             public void publish(LogRecord record) {
    [javac]                         ^
    [javac]     (see http://errorprone.info/bugpattern/UnsynchronizedOverridesSynchronized)
    [javac]   Did you mean 'public synchronized void publish(LogRecord record) {'?

comment:15 Changed 3 years ago by michael2402

No. This is a false positive. The parent method is not even synchronized, only the parent of the parent is.

The Idea behind it is, that you do not need to lock the object for the log level check.

comment:16 Changed 3 years ago by Don-vip

Summary: [Patch] Clean up program startup (parameters, logging)Clean up program startup (parameters, logging)

comment:18 Changed 3 years ago by Don-vip

Resolution: fixed
Status: reopenedclosed

I agree it's false positive, let Google fix this one.

comment:19 Changed 2 years ago by simon04

In 11165/josm:

see #13318 - more concise logging output

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.