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

mps3an547 doc update #14990

Merged
merged 1 commit into from
Nov 29, 2024
Merged

mps3an547 doc update #14990

merged 1 commit into from
Nov 29, 2024

Conversation

anjiahao1
Copy link
Contributor

@anjiahao1 anjiahao1 commented Nov 29, 2024

Summary

mps3an547 doc update

In the new version of QEMU (9.20), the UART RX interrupt and TX interrupt have been swapped. Adjustments need to be made using menuconfig.

see it qemu/qemu@5a558be

Impact

noting

Testing

use latest qemu

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Size: XS The size of the change in this PR is very small labels Nov 29, 2024
@github-actions github-actions bot added Size: S The size of the change in this PR is small and removed Size: XS The size of the change in this PR is very small labels Nov 29, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 29, 2024

[Experimental Bot, please feedback here]

No, this PR likely does not meet the NuttX requirements based on the information provided. While it mentions a documentation update, it lacks crucial details. Here's why:

  • Insufficient Summary: "mps3an547 doc update" is too vague. What specifically was updated in the documentation? What problem did the update address? What part of the mps3an547 is documented?
  • Missing Impact Details: While "noting" suggests no impact, it's best to explicitly state "NO" for each impact category. Even for documentation changes, describe what documentation changed.
  • Inadequate Testing: "use latest qemu" isn't sufficient. What tests were run? Provide specific commands used and the relevant output demonstrating the documentation's correctness (e.g., before/after screenshots if it's a visual change, or commands that now work due to the improved docs). The "Testing logs before change" and "Testing logs after change" sections are empty, which is a major red flag.

To make this PR acceptable, the author needs to:

  1. Expand the Summary: Explain the reason for the documentation update, the specific part of the mps3an547 being documented, and the nature of the changes made to the documentation.
  2. Clarify Impact: Explicitly state "NO" for all impact categories where applicable. For the "Impact on documentation" section, specify which documentation files were changed and how.
  3. Provide Detailed Testing Information: Describe the specific tests conducted on QEMU. Include the QEMU version, the NuttX configuration used, the commands executed to verify the documentation, and the output of those commands both before and after the change. Screenshots can be helpful for visual changes. Even if the change is just documentation, demonstrating its practical value through example commands and their outputs makes the PR much stronger.

@xiaoxiang781216 xiaoxiang781216 merged commit 68ac081 into apache:master Nov 29, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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