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

syslog/channel: rename syslog_channel() to syslog_channel_register() #13571

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

anchao
Copy link
Contributor

@anchao anchao commented Sep 23, 2024

Summary

syslog/channel: rename syslog_channel() to syslog_channel_register()

Change syslog API naming more reasonable:

  1. rename syslog_channel() to syslog_channel_register()
  2. rename syslog_channel_remove() to syslog_channel_unregister()

Signed-off-by: chao an [email protected]

Impact

N/A

Testing

ci-check

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Arch: arm Issues related to ARM (32-bit) architecture Size: S The size of the change in this PR is small labels Sep 23, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 23, 2024

[Experimental Bot, please feedback here]

This PR appears to meet the basic NuttX requirements, but it's missing some key information:

Summary:

  • Why change is necessary? The summary mentions making the naming "more reasonable," but doesn't explain why the current naming is unreasonable or what benefits the new naming brings.
  • What functional part of the code is being changed? While "syslog/channel" is mentioned, a slightly more detailed description of this part of the codebase would be helpful.

Impact:

  • While marking everything as "N/A" might be technically correct, it's best to be explicit. For example:
    • Impact on user: Will users need to update their applications to use the new function names? If so, is this change backwards compatible?
    • Impact on documentation: Even if the changes are small, the documentation likely needs updating to reflect the new function names.

Testing:

  • "ci-check" is not enough information. Provide details about the build host(s) and target(s) used for testing.
  • Include relevant testing logs before and after the change. This helps demonstrate the issue the PR addresses and proves that the change works as intended.

In short, the PR needs more context and details to be considered complete.

Change syslog API naming more reasonable:

1. rename syslog_channel() to syslog_channel_register()
2. rename syslog_channel_remove() to syslog_channel_unregister()

Signed-off-by: chao an <[email protected]>
@xiaoxiang781216 xiaoxiang781216 merged commit a525116 into apache:master Sep 24, 2024
29 checks passed
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 Area: Documentation Improvements or additions to documentation 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