Simplify logging #233
Simplify logging #233
Conversation
… Maven logging. log message string construction to address.
…unique per test to avoid test lock-ups on Windows due to locked files (also enables parallel testing)
…variable violates thread safety
| @@ -386,7 +386,7 @@ Start out with with adding the above steps to your project, next paste this **gi | |||
| <property name="buildUserEmail" value="${git.build.user.email}"/> | |||
| <property name="buildTime" value="${git.build.time}"/> | |||
| <property name="buildHost" value="${git.build.host}"/> | |||
| <porperty name="buildVersion" value="${git.build.version}"/> | |||
| <property name="buildVersion" value="${git.build.version}"/> | |||
There was a problem hiding this comment.
funny one, thanks (and for all the other typos too, thanks)
| @@ -69,16 +48,18 @@ | |||
| </scm> | |||
|
|
|||
| <properties> | |||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | |||
| <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding> | |||
There was a problem hiding this comment.
Could be even shortened to <encoding>UTF-8</encoding> if I remember correctly.
|
I like most of this PR but (maybe just as a note to myself) this makes the modularisation effort (see #92 for the discussion) even harder as now everything is dependent on maven. |
…plugin-specific exception to address Michael comment about modularization (git-commit-id#92)
|
Thank you for a thorough review! I've addressed the comments above. Namely, logging bridges now are improved. In addition, I've refactored exception handling to favor modularization, removed few remaining static collection constructors, added project encoding handling to avoid hard-coded charsets. It turns out property="session" removed earlier is needed. This case stresses the need for proper integration testing: I only got this one with manual testing. It should have been done by integration tests. Please, review and merge. Thank you! |
|
Is there any other outstanding issues with this branch which we need to consider? |
|
I'll need to find time to review again – we don't want to make the modularisation effort harder, as mentioned by @mvitz. Please bear with me :) |
|
OK! |
|
Konrad, please, can you take a look at this pull request? Thank you! |
|
Konrad, what are the chances of seeing this merged? |
|
I'll review again today or tomorrow, was traveling and now sick. Thanks for your patience |
|
Thank you! |
|
Konrad, how do I interpret the situation with this pull? You're saying you'll merge it, but months later it is still not merged. |
|
Please don't assume malice where none is intended – i was very sick for a few weeks and then had work travel, was unable to catch up on all things. Thanks for the ping here (that was all that's needed) – will go over it now and merge if all good :) |
|
Minor nitpicks but no reason to delay merging, thanks again for your patience :) |
|
Thank you for merging! And for useful review! Now the next step - release ;) |
|
By the way, your above comments are not nitpicking - there were few things in the pull which were touched by me "passing by", minor refactorings, which strictly speaking should have been separated from this pull. |
| void setVerbose(boolean verbose); | ||
|
|
||
| /* | ||
| The following borrowed from SLF4J Logger.java |
There was a problem hiding this comment.
I had to check license compatibility... seems we're good though - MIT
This pull contains the following changes (in addition to logging simplification itself)