Better log output to console #21
Conversation
|
I cant correlate the error to my PR. Any help? cc/ @basil |
|
I think I see the problem here, which is that you're referring to a variable named |
|
Oh I totally missed that one! |
chipironcin
force-pushed
the
jorge_improve_log
branch
from
346618a
to
ae92ff3
Compare
chipironcin
force-pushed
the
jorge_improve_log
branch
from
e0aaa7f
to
85d38e5
Compare
chipironcin
force-pushed
the
jorge_improve_log
branch
from
5e36af0
to
d9d842f
Compare
|
I finally made it work. |
| @@ -128,24 +128,29 @@ private void findText(Run<?, ?> run, FilePath workspace, TaskListener listener) | |||
| boolean foundText = false; | |||
|
|
|||
| if (alsoCheckConsoleOutput) { | |||
| logger.println("Checking console output"); | |||
| // Do not mention the pattern we are looking for to avoid false positives | |||
| logger.println("Looking for a specific pattern in the console output"); | |||
There was a problem hiding this comment.
Looks great! Thanks for doing this. There's only one minor comment I have. The wording of the message "Looking for a specific pattern in the console output" seems potentially confusing to the end user. To me, this message raises the question: "What is that specific pattern?" Of course, we can't tell the user what the specific pattern is, because that would lead to a false positive, as your comment states. How about if we changed the logic such that instead of printing "Looking for a specific pattern in the console output", we printed "Searched for pattern X in the console output" after performing the search? That way, we'd avoid the false positive. We'd probably want to change the behavior for files to do the same thing, for consistency. Then we'd be able to print the pattern in the message in both cases. Let me know what you think of this idea. If you don't think it's worth the additional work, then I'm fine with this change as-is.
There was a problem hiding this comment.
Thanks for taking the time on thinking about this!
I also went through this a couple of times, I really wanted to show the pattern but felt it required some user feedback BEFORE starting looking.
I was thinking on saving the console log, print message and then do the search on the saved content but the size might be too big in memory in certain situations.
I will try to apply your suggestion, adding a message BEFORE the search so the user knows what is happening.
|
Expected changes required |
chipironcin
force-pushed
the
jorge_improve_log
branch
from
3f2634e
to
1341ff8
Compare
|
@basil when you have a chance would you mind checking the latest changes? I added a new log line before the search explaining a search is about to start. Hope you like the approach |
|
cool, I would welcome this. @basil do you plan to merge this? |
This change improves what the plugin logs to the console to it is easier for the user to identify what the plugin is looking for and where.
Updated the tests