You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "XuNeo (via GitHub)" <gi...@apache.org> on 2024/10/15 06:50:28 UTC
[PR] mm/Kconfig: adjust config options order and fix typos [nuttx]
XuNeo opened a new pull request, #14294:
URL: https://github.com/apache/nuttx/pull/14294
*Note: Please adhere to [Contributing Guidelines](https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md).*
## Summary
1. Group related configurations together.
2. Use `if endif` for GRAN(Granule Allocator) module.
3. Fix typos and misleading hints
4. Add necessary depends on
- MM_HEAP_BIGGEST_COUNT depends on !FS_PROCFS_EXCLUDE_MEMDUMP
- FS_PROCFS_EXCLUDE_MEMPOOL depends on MM_HEAP_MEMPOOL_THRESHOLD > 0 -
*Update this section with information on why change is necessary,
what it exactly does and how, if new feature shows up, provide
references (dependencies, similar problems and solutions), etc.*
## Impact
Should be minimal since most of the change is to adjust order.
*Update this section, where applicable, on how change affects users,
build process, hardware, documentation, security, compatibility, etc.*
## Testing
*Update this section with details on how did you verify the change,
what Host was used for build (OS, CPU, compiler, ..), what Target was
used for verification (arch, board:config, ..), etc. Providing build
and runtime logs from before and after change is highly appreciated.*
CI test pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] mm/Kconfig: adjust config options order and fix typos [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #14294:
URL: https://github.com/apache/nuttx/pull/14294#issuecomment-2413058147
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
## NuttX PR Requirements Review
This PR appears **incomplete** based on the provided information.
**Here's a breakdown:**
**Summary:**
* **Insufficient Information:** While the summary outlines the changes made, it lacks crucial context.
* **Why are these changes necessary?**
* Are there specific bugs being fixed?
* Is this enhancing maintainability or readability?
* Is there a performance improvement goal?
* **How do the 'depends on' additions impact functionality?** Explain the reasoning behind these dependencies.
* **Missing Issue References:** Are there related NuttX or NuttX Apps issues that this PR addresses? If so, provide links.
**Impact:**
* **Too Vague:** "Should be minimal" is not descriptive enough. Address each impact point specifically:
* **User Impact:** Will users notice any functional changes? Will any configuration options change?
* **Build Impact:** Will build times change? Are new build dependencies introduced?
* **Hardware Impact:** Will any existing hardware configurations break?
* **Documentation Impact:** Does the documentation need updating to reflect these changes?
* **Security Impact:** Do these changes have any implications for the security of NuttX?
* **Compatibility Impact:** Will this break compatibility with older NuttX versions or configurations?
* **Consider Edge Cases:** Even if the impact *seems* minimal, think about less common scenarios that might be affected.
**Testing:**
* **Missing Details:**
* **Host Information:** Provide specifics about your build host (OS version, compiler version, etc.).
* **Target Information:** Clearly state the target architecture, board, and configuration used for testing.
* **Logs Required:** Include actual build and runtime logs from both *before* and *after* the changes. This provides concrete evidence that your changes work as intended.
* **CI Passing Is Not Enough:** While CI passing is good, it doesn't replace detailed testing documentation.
**Recommendations:**
1. **Expand the Summary:** Provide a more comprehensive explanation of the changes and their motivations.
2. **Address All Impact Points:** Be specific and thorough in describing the potential impact of this PR.
3. **Provide Detailed Testing Information:** Include specific host/target details and the requested logs.
**By addressing these points, you'll make your PR much stronger and easier for reviewers to understand and approve.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] mm/Kconfig: adjust config options order and fix typos [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #14294:
URL: https://github.com/apache/nuttx/pull/14294
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org