Skip to content

task: use get_task_name where possible#13771

Merged
xiaoxiang781216 merged 1 commit into
apache:masterfrom
XuNeo:add-get-task-name
Oct 1, 2024
Merged

task: use get_task_name where possible#13771
xiaoxiang781216 merged 1 commit into
apache:masterfrom
XuNeo:add-get-task-name

Conversation

@XuNeo

@XuNeo XuNeo commented Oct 1, 2024

Copy link
Copy Markdown
Contributor

Summary

Introduce get_task_name to streamline code by replacing (#if #else #endif) with a cleaner implementation.

Impact

For debug log, instead of printing nothing, now it will print "noname".

Testing

CI test should be enough for this change.

@github-actions github-actions Bot added Arch: arm Issues related to ARM (32-bit) architecture Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Arch: xtensa Issues related to the Xtensa architecture Area: Board support Board support issues Area: File System File System issues Area: OS Components OS Components issues Area: BINFMT Size: M The size of the change in this PR is medium labels Oct 1, 2024
@nuttxpr

nuttxpr commented Oct 1, 2024

Copy link
Copy Markdown

[Experimental Bot, please feedback here]

This PR appears incomplete based on the NuttX requirements.

Here's why and what's missing:

  • Summary:

    • What functional part of the code is being changed? Be specific. Mention the affected module(s) or file(s).
    • How does the change exactly work? Explain the mechanism of get_task_name and how it replaces the #if/#else logic.
  • Impact:

    • Impact on user: While seemingly minor, changing debug output can impact users. Be explicit:
      • Will existing user scripts break?
      • Is "noname" the appropriate output, or should it be configurable?
    • Impact on documentation: Even small changes often need documentation updates:
      • Is get_task_name documented somewhere?
      • Do existing code comments need adjustment?
  • Testing:

    • CI is NOT enough. You need to provide specific local testing details:
      • Build Host: At least one OS/CPU/compiler combination
      • Target: At least one architecture/board/configuration
    • Testing logs: While "before" logs might be trivial, you MUST include "after" logs demonstrating the change in output.

In short: Don't assume reviewers understand the context. Provide enough detail to make the PR's value and correctness immediately clear.

Comment thread include/nuttx/sched.h Outdated
Signed-off-by: xuxingliang <xuxingliang@xiaomi.com>
@XuNeo XuNeo force-pushed the add-get-task-name branch from f26c708 to fc1614c Compare October 1, 2024 04:12
@xiaoxiang781216 xiaoxiang781216 merged commit 7044b10 into apache:master Oct 1, 2024
@XuNeo XuNeo deleted the add-get-task-name branch October 1, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: arm Issues related to ARM (32-bit) architecture Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Arch: xtensa Issues related to the Xtensa architecture Area: BINFMT Area: Board support Board support issues Area: File System File System issues Area: OS Components OS Components issues Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants