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.

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

Correct log files for Qconnect #440

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

huavanthong
Copy link
Collaborator

@huavanthong huavanthong commented Feb 12, 2025

Hi Thomas,
Cc Holger

I have updated the option on QConnect.
Please help me review it.
#438

Thank you and best regards,
Thong Hua

@@ -38,9 +38,8 @@ for target_dir in "${value_array[@]}"; do
if [ $? -ne 0 ]; then
real_src_path=$(realpath $(dirname $source_log_file))
echo -e "\e[31mError:\e[0m No files to copy files from $real_src_path"
echo "Please correct the directory to contain all log files."
exit 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

The wording is a bit curious. Files are missing. This is interpreted as error. OK so far. But do we really need an additional hint that something needs to be corrected? I don't think so.

How about a shorter version:

Error: No log files found in '$real_src_path'.

This should be enough.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi Holger,

I think we need to warn the user to pay attention to additional test log files.
Therefore, I will change "Error" to "Warning" with a shorter version, as you mentioned.

Please help me review it and let me know if you have any concerns.

Best regards,
Thong Hua

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi Thong,

fine to me.

Best regards

Holger

Copy link
Collaborator

@HolQue HolQue left a comment

Choose a reason for hiding this comment

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

Wording only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants