Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Stop the running terminal and then Clean and close all terminal tabs. #1049

Merged
merged 8 commits into from
Nov 7, 2024
Merged

Stop the running terminal and then Clean and close all terminal tabs. #1049

merged 8 commits into from
Nov 7, 2024

Conversation

anusreelakshmi934
Copy link
Contributor

Fixes #929

turkeylurkey
turkeylurkey reviewed Oct 23, 2024
View reviewed changes
Copy link
Member

@turkeylurkey turkeylurkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should perform all operations before you print "Exit."

turkeylurkey
turkeylurkey previously approved these changes Oct 30, 2024
View reviewed changes
turkeylurkey
turkeylurkey requested changes Nov 1, 2024
View reviewed changes
Copy link
Member

@turkeylurkey turkeylurkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you show us the impact of using Logger rather than TestUtils.printTrace()?

@anusreelakshmi934
Copy link
Contributor Author

Could you show us the impact of using Logger rather than TestUtils.printTrace()?

When using TestUtils.TraceSevLevel.INFO it shows in the logs like this as INFO
image

Also while using LOGGER.warn it shows as a warning in the log.
image

So I thought it would be better to go with warning. @turkeylurkey Do you have any other thoughts on this?

mrglavas
mrglavas approved these changes Nov 6, 2024
View reviewed changes
Copy link
Contributor

@mrglavas mrglavas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed and reviewed with the team. Looks good.

@anusreelakshmi934 anusreelakshmi934 merged commit 9272646 into OpenLiberty:main Nov 7, 2024
2 of 3 checks passed
This was referenced Nov 14, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turkeylurkey turkeylurkey turkeylurkey approved these changes

@mrglavas mrglavas mrglavas approved these changes

@aparnamichael aparnamichael Awaiting requested review from aparnamichael

@TrevCraw TrevCraw Awaiting requested review from TrevCraw

@dessina-devasia dessina-devasia Awaiting requested review from dessina-devasia

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Custom start is getting stuck while trying to access messages.log
4 participants
@anusreelakshmi934 @mrglavas @turkeylurkey @dessina-devasia