Skip to content

note: change sched_note_counter to a macro#13948

Merged
xiaoxiang781216 merged 1 commit into
apache:masterfrom
Gary-Hobson:note-counter
Oct 8, 2024
Merged

note: change sched_note_counter to a macro#13948
xiaoxiang781216 merged 1 commit into
apache:masterfrom
Gary-Hobson:note-counter

Conversation

@Gary-Hobson

Copy link
Copy Markdown
Contributor

Summary

Change inline functions to macros to unify code style

Impact

none

Testing

sim/note

Signed-off-by: yinshengkai <yinshengkai@xiaomi.com>
@github-actions github-actions Bot added the Size: S The size of the change in this PR is small label Oct 8, 2024
@nuttxpr

nuttxpr commented Oct 8, 2024

Copy link
Copy Markdown

[Experimental Bot, please feedback here]

This PR needs more information to meet the NuttX requirements. Here's a breakdown:

What's Missing & How to Improve:

  • Summary:

    • Specificity: "Unify code style" is vague. Which inline functions? In what part of the codebase (drivers, kernel, etc.)?
    • Justification: Why is changing inline functions to macros necessary? Is it just about style, or are there performance implications?
    • Example: "Change inline functions for semaphore operations in sched/ to macros for consistency with other synchronization primitives."
  • Impact:

    • Overly General: Even if there's no user-facing impact, be specific.
    • Example:
      • "Impact on build: None."
      • "Impact on hardware: No changes to architecture or board support."
      • "Impact on documentation: No updates required."
  • Testing:

    • Insufficient Detail:
      • Build Host: State your OS, CPU architecture, and compiler version.
      • Target: "sim/note" isn't clear. Which simulator? What's the "note" configuration?
    • Missing Logs: Provide actual testing logs (even if they're short) to demonstrate the change's effect.

Example Improvements:

## Summary

This PR changes inline functions for semaphore management in `sched/semaphore/` to macros. This aligns the code style with other synchronization primitives in NuttX and improves code readability.

## Impact

* Is new feature added? NO
* Is existing feature changed? NO
* Impact on user: NO
* Impact on build: NO
* Impact on hardware: NO
* Impact on documentation: NO
* Impact on security: NO
* Impact on compatibility: NO

## Testing

I confirm that changes are verified on my local setup and work as intended:

* Build Host(s): Linux (Ubuntu 20.04), x86_64, GCC 9.4.0
* Target(s):  sim/qemu-arm (Cortex-M3 configuration)

Testing logs before change: 

(Provide a snippet of relevant log output)


Testing logs after change:

(Provide a snippet showing the effect of the change)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants