Fix "Running Changeset: " logs written directly to stdout instead of using the maven logger #4157
Conversation
|
Initial/Pre-Review Thoughts The change looks right to me. The UIService is there to adapt to whatever "UI" a particular integration has, and for maven their UI is the logs. It may make for some duplicate log messages, but the fix for that is larger and should be handled in a separate PR at some point, likely thinking about UI vs. logs more in general. Questions I have:
Potential risks:
What could make the full review difficult:
|
|
Hello @mensinda - PR #4152 was merge last week and coincidentally it has a method with the same that you used. Could you merge your changes to it? I believe your code would be added around line 633 -: https://github.com/liquibase/liquibase/pull/4152/files#diff-bbe8f062b4c3acf56905d187f674c41575cc99e3b114267095848997859f5926R633 ? |
This way the "Running Changeset: " logs are written with the maven logger and not directly to stdout.
|
Hello @filipelautert, I have updated the PR. |
filipelautert
left a comment
There was a problem hiding this comment.
Thanks for the fix @mensinda !
XDelphiGrl
left a comment
There was a problem hiding this comment.
This PR improves Liquibase Maven Plugin logging by using the the Maven Log UI instead of stdout for writing some messages.
- Functional and test harness executions passing. There were some failures in an earlier test harness run, but those are resoleved here.
- No additional testing required.
APPROVED
Impact
I am categorizing this as a bugfix and not an enhancement, since I don't think that using the default
ConsoleUIService(which writes to stdout) was intended.Description
With this PR, the "Running Changeset: " logs are written with the maven logger and not directly to stdout.
Things to be aware of
Impact on the codebase: minimal
Things to worry about
Nothing
Additional Context