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

Fix Kconfig style #15147

Merged
merged 1 commit into from
Dec 11, 2024
Merged

Fix Kconfig style #15147

merged 1 commit into from
Dec 11, 2024

Conversation

simbit18
Copy link
Contributor

Summary

Remove spaces from Kconfig files
Add TABs

Impact

none

Testing

Remove spaces from Kconfig files
Add TABs
@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Arch: xtensa Issues related to the Xtensa architecture Size: S The size of the change in this PR is small labels Dec 11, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 11, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides a brief summary, it lacks crucial details.

Here's what's missing:

  • Summary:

    • WHY: The summary states what was changed (removing spaces and adding tabs), but not why. What problem did this solve? Was it for code style consistency, fixing a parsing issue, or something else?
    • HOW: It mentions adding tabs, but doesn't specify where or how many. Consistency is key in code style, so the specifics are important. What was the standard adopted?
    • Related Issues: Any related NuttX issues should be linked. Even if there isn't a directly related issue, it's good practice to explain why one wasn't created if the change addresses a bug.
  • Impact: While stating "none" is concise, it's unlikely there's zero impact. Consider these:

    • Build: Did the build system need any adjustments to handle the changes to the Kconfig files? Even seemingly minor changes can sometimes have ripple effects.
    • Documentation: If this change affects how Kconfig files are formatted, the coding style guidelines might need updating to reflect the new standard.
  • Testing: This section is completely empty. It's crucial to provide evidence that the changes work as intended. At minimum, include:

    • Build Host: Your development environment (OS, CPU architecture, compiler version).
    • Target(s): The architectures and boards where the code was tested.
    • Logs: "Before" and "after" logs aren't strictly required for trivial whitespace changes, but demonstrating that the configuration system still functions correctly after the modifications is essential. A simple successful build log would suffice.

In short, the PR needs significantly more detail to meet the NuttX requirements. Focus on explaining the why, how, and providing evidence of testing.

@xiaoxiang781216 xiaoxiang781216 merged commit e4705e7 into apache:master Dec 11, 2024
27 checks passed
@simbit18 simbit18 deleted the simbit18-kconfig branch December 12, 2024 08:52
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: xtensa Issues related to the Xtensa architecture 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.

4 participants