Repository navigation
#2385: Cleanup of IdeLogEntry, IdeLogListener and IdeGuiLogListener - #2639
NikoFoutsitzis wants to merge 40 commits into
Conversation
|
ConsolePanelTest: In the testmessage "Hello World!" was missing "!" so the test failed. |
…and-IdeGuiLogListener
…LogListener-and-IdeGuiLogListener' into cleanup/2385-IdeLogEntry-and-IdeLogListener-and-IdeGuiLogListener
Coverage Report for CI Build 39765794280Coverage decreased (-0.02%) to 74.981%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions20 previously-covered lines in 5 files lost coverage.
Coverage Stats💛 - Coveralls |
quando632
left a comment
There was a problem hiding this comment.
Nice work @NikoFoutsitzis! Passing the whole IdeLogEntry through onLog simplifies every listener, and IdeGuiLogListener now fixes the duplicate output at the root.
The Instant timestamp with a constant formatter and the recording ConsoleController that avoids the JavaFX toolkit in the test are clean touches as well.
…ogListener.java Co-authored-by: quando632 <quang-hieu.do@capgemini.com>
Co-authored-by: quando632 <quang-hieu.do@capgemini.com>
…ConsolePanelTest.java Co-authored-by: quando632 <quang-hieu.do@capgemini.com>
…LogListener-and-IdeGuiLogListener' into cleanup/2385-IdeLogEntry-and-IdeLogListener-and-IdeGuiLogListener
…LogListener-and-IdeGuiLogListener' into cleanup/2385-IdeLogEntry-and-IdeLogListener-and-IdeGuiLogListener
JoelAdbu
left a comment
There was a problem hiding this comment.
@NikoFoutsitzis thank you for the PR. The cleanup looks good overall, just two minor suggestions. It would also be worth adding a small test for IdeLogEntry covering the constructor edge cases
…stenerTest.java Co-authored-by: JoelAdbu <joel.adabunu@capgemini.com>
…LogListener-and-IdeGuiLogListener' into cleanup/2385-IdeLogEntry-and-IdeLogListener-and-IdeGuiLogListener
…Entry-and-IdeLogListener-and-IdeGuiLogListener
…and-IdeGuiLogListener
quando632
left a comment
There was a problem hiding this comment.
Thanks @NikoFoutsitzis, all the earlier points are in and the merge onto the reworked console was done cleanly.
…ervice.java Co-authored-by: quando632 <quang-hieu.do@capgemini.com>
…and-IdeGuiLogListener
quando632
left a comment
There was a problem hiding this comment.
Thanks @NikoFoutsitzis, the null check, the duplicate error event and the static context cleanup are all sorted, nice work!
The merge onto the event-bus console is clean apart from one spot where the original entry got lost again.
…LogListener-and-IdeGuiLogListener' into cleanup/2385-IdeLogEntry-and-IdeLogListener-and-IdeGuiLogListener
…and-IdeGuiLogListener
…and-IdeGuiLogListener
…and-IdeGuiLogListener
quando632
left a comment
There was a problem hiding this comment.
Thanks @NikoFoutsitzis, the listener now forwards the original entry, CI is green again and the null-message case is covered, nice work!
Only two small test touch-ups are left.
…and-IdeGuiLogListener
…and-IdeGuiLogListener
…and-IdeGuiLogListener
…and-IdeGuiLogListener
This PR cleanup #2385
Implemented changes:
Testing instructions
Please add conscise, understandable instructions on how a reviewer can test/verify the functionality of your contribution here: