Skip to content

hijack: don't collect logger if SOF_LOGGING=none#1018

Merged
fredoh9 merged 1 commit intothesofproject:mainfrom
fredoh9:fix/sof_logging_none
Apr 7, 2023
Merged

hijack: don't collect logger if SOF_LOGGING=none#1018
fredoh9 merged 1 commit intothesofproject:mainfrom
fredoh9:fix/sof_logging_none

Conversation

@fredoh9
Copy link
Copy Markdown
Contributor

@fredoh9 fredoh9 commented Apr 7, 2023

SOF_LOGGING=none means no logger log. This condition must be checked before the other conditions.

SOF_LOGGING=none means no logger log. This condition must be checked
before the other conditions.

Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
@fredoh9 fredoh9 requested a review from a team as a code owner April 7, 2023 05:06
Comment thread case-lib/hijack.sh
if is_ipc4 && is_firmware_file_zephyr; then
local mtraceBin; mtraceBin=mtrace-reader.py
if [ "$SOF_LOGGING" != 'none' ] && is_ipc4 && is_firmware_file_zephyr; then
local mtraceBin; mtraceBin=mtrace-reader.py
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Whitespace change: Good thing this isn't Python. Rest of the change isn't an issue so I can still approve.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I hate unrelated whitespace fixes because they break git cherry-pick, merge, blame etc. but in this case the change is so close that git would conflict anyway.

@fredoh9 fredoh9 requested review from keqiaozhang and marc-hb April 7, 2023 17:01
marc-hb added a commit to marc-hb/sof-test that referenced this pull request Apr 7, 2023
…ition

Fixes warning found in
https://github.com/thesofproject/sof-test/actions/runs/4635603962/jobs/8202801248?pr=1018
testing PR thesofproject#1018

See inline comments. Fixes commit 0ccb4c8 ("hijack.sh: more
generic way to disable unknown $logfile shellcheck")

Different shellcheck versions may warn about this fake definition
differently but life is too short to compare them, just disable both
SC2154 and SC2269

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Copy link
Copy Markdown
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

Fix for unrelated shellcheck warning in https://github.com/thesofproject/sof-test/actions/runs/4635603962/jobs/8202801248?pr=1018 submitted in separate PR #1020

@marc-hb marc-hb requested review from a team, greg-intel, plbossart and ranj063 April 7, 2023 17:27
fredoh9 pushed a commit that referenced this pull request Apr 7, 2023
…ition

Fixes warning found in
https://github.com/thesofproject/sof-test/actions/runs/4635603962/jobs/8202801248?pr=1018
testing PR #1018

See inline comments. Fixes commit 0ccb4c8 ("hijack.sh: more
generic way to disable unknown $logfile shellcheck")

Different shellcheck versions may warn about this fake definition
differently but life is too short to compare them, just disable both
SC2154 and SC2269

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@fredoh9 fredoh9 merged commit c367f32 into thesofproject:main Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants