Add ability to log exceptions as error.#235
Open
fandfisf wants to merge 2 commits intojcabi:masterfrom
Open
Conversation
This chage does not break backwords compatibility.
Author
|
Please help. The appVeyor check failed because maven path was set windows style. I have not checked in anything to change build platform/OS |
|
@fandfisf Let me find a reviewer for this pull request, thanks for submitting it |
|
@original-brownbear please review this PR |
| new MethodLogger.Marker(point, annotation); | ||
| this.running.add(marker); | ||
| int level = annotation.value(); | ||
| final int exceptionLevel = this.decideExceptionLevel(annotation); |
There was a problem hiding this comment.
@fandfisf we don't generally use compound method variable names, let's just leave this as level :)
| */ | ||
| private int decideExceptionLevel(final Loggable annotation) { | ||
| int returnValue = annotation.value(); | ||
| if (annotation.exceptionsAsError()) { |
There was a problem hiding this comment.
@fandfisf very minor but you got two spaces behind the = here, let's fix the formatting and make it just one space :)
Author
There was a problem hiding this comment.
Is this before the "Loggable.ERROR"? Will fix that most likely this week-end.
|
@fandfisf thanks for the contribution. Removing that pid file def makes sense I think :) Only two minor points from my end. |
Author
|
Will create an issue and will link to the PR description as you suggest. By the way, what is |
Author
|
Created issue #236 |
|
Any update on this? |
Member
|
@rultor merge |
Contributor
Contributor
@fandfisf @yegor256 Oops, I failed. You can see the full log here (spent 5min) |
Member
|
@rultor merge |
Contributor
Contributor
@fandfisf @yegor256 Oops, I failed. You can see the full log here (spent 7min) |
Member
|
@rultor merge |
Contributor
Contributor
@fandfisf @yegor256 Oops, I failed. You can see the full log here (spent 8min) |
Member
|
@fandfisf it seems that your code is not "clean" enough. Please, run your branch with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See issue #236
This chage does not break backwords compatibility. The most common case is to log entry/exit at either INFO or DEBUG level, however, exceptions are usually logged as "ERROR". This allows the default behaviour to remain unchanged (exceptions logged at the same level as entry and exit) however for code-bases that do not use exceptions for flow control but as real exceptions, it allows them to be logged as ERROR. Hope this helps.
I also deleted a "pid" file in the root as it looked like an accidental check in. Pease discard the change if that is no the case.
Thanks!