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

can: Add g_ prefix to can_dlc_to_len and len_to_can_dlc. #13524

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

OceanfromXiaomi
Copy link
Contributor

@OceanfromXiaomi OceanfromXiaomi commented Sep 18, 2024

Summary

Add g_ prefix to can_dlc_to_len and len_to_can_dlc.

Impact

Add g_ prefix to can_dlc_to_len and len_to_can_dlc to follow NuttX coding style conventions for global symbols, improving code readability and maintainability.

Testing

detail: Add g_ prefix to can_dlc_to_len and len_to_can_dlc to
follow NuttX coding style conventions for global symbols,
improving code readability and maintainability.

Signed-off-by: zhaohaiyang1 <zhaohaiyang1@xiaomi.com>
@lupyuen
Copy link
Member

lupyuen commented Sep 18, 2024

[Experimental Bot, please feedback here]

Yes, the PR appears to meet NuttX requirements.

Here's a breakdown:

Meets Requirements:

  • Summary: Clearly states the "why" (coding style) and "what" (adding 'g_' prefix to functions).
  • Impact:
    • Accurately identifies the limited scope of the change (coding style only).
    • Correctly states there's no user, build, hardware, security, or compatibility impact.
    • Could benefit from explicitly stating if documentation needs an update.
  • Testing:
    • While the template is provided, the lack of actual test logs is a significant omission. This needs to be filled in.

Recommendations for Improvement:

  • Testing: Provide specific test logs. This is crucial for reviewers to understand how the change was validated. Include:
    • Commands used to build and run tests.
    • Relevant output snippets that demonstrate the change's effect (or lack thereof, since it's stylistic).
  • Impact - Documentation: Specify if the coding style change requires a corresponding update to any documentation (e.g., coding guidelines).

Concise Feedback:

The PR summary and impact assessment meet NuttX requirements. However, providing detailed test logs is essential for a complete and reviewable PR.

Copy link
Contributor

@cederom cederom left a comment

Choose a reason for hiding this comment

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

Thank you :-)

@xiaoxiang781216 xiaoxiang781216 merged commit f76c2ed into apache:master Sep 18, 2024
28 checks passed
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.

5 participants