New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix @mensinda !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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