Skip to content

[bp-15102} update documentation#15133

Merged
xiaoxiang781216 merged 3 commits into
apache:releases/12.8from
jerpelea:bp-15102
Dec 11, 2024
Merged

[bp-15102} update documentation#15133
xiaoxiang781216 merged 3 commits into
apache:releases/12.8from
jerpelea:bp-15102

Conversation

@jerpelea

Copy link
Copy Markdown
Contributor

Summary

this patch includes
#15102
#15121
#15122

Impact

RELEASE

Testing

CI

xiaoxiang781216 and others added 3 commits December 11, 2024 08:42
since the basepri is always used without any configuraion

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
This is a follow-up to 366c8a5 (PR-15102).

* Documentation/guides/zerolatencyinterrupts.rst
  (Title): Make title case consistent.
  (Getting Back into the Game, Maskable Nested Interrupts): Clarify discussion
   about priorities.
  (Cortex-M3/4 Implementation): The first half of a sentence was deleted in
   366c8a5 because the Kconfig that was described there no longer exists:
   CONFIG_ARMV7M_USEBASEPRI. Write a new beginning for this sentence that
   matches current implementation.
  (Disabling the High Priority Interrupt): Change "cannot" to "must not be
   allowed to" to improve clarity.
  (Configuring High Priority Interrupts): Change "to NVIC" to "in NVIC" to
   improve clarity.
Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@github-actions github-actions Bot added Area: Documentation Improvements or additions to documentation Size: S The size of the change in this PR is small labels Dec 11, 2024
@jerpelea jerpelea changed the title [Bp-15102} update documentation [bp-15102} update documentation Dec 11, 2024
@nuttxpr

nuttxpr commented Dec 11, 2024

Copy link
Copy Markdown

[Experimental Bot, please feedback here]

No, this PR summary does not adequately meet the NuttX requirements. While it references other PRs, it fails to provide the necessary context and details required by the template.

Here's why it's insufficient and how it could be improved:

  • Summary: Simply listing PR numbers doesn't explain why these changes are necessary, what they change, or how they work. The summary should be self-contained and describe the overall purpose and functionality of the combined changes.
  • Impact: Stating "RELEASE" doesn't provide any specific information. Each impact category (user, build, hardware, documentation, security, compatibility) should be addressed individually with "YES" or "NO" and a description if "YES". For a release, many of these are likely to be "YES" and require explanation.
  • Testing: "CI" is insufficient. While CI testing is important, the template requires specific details about the local testing environment (host and target) and logs demonstrating the behavior before and after the change. Relying solely on CI results doesn't offer enough evidence of thorough testing, especially for a release.

How to improve this PR summary:

  1. Consolidate the changes: If these three PRs are logically related and meant to be merged together, the ideal approach would be to squash them into a single, well-structured PR. This simplifies review and makes tracking changes easier.

  2. Provide a comprehensive summary: Explain the overall goal of the combined changes. What problem are they solving, or what new functionality are they introducing? Briefly describe the core modifications made in each of the original PRs within this unified summary.

  3. Detail the impact: Go through each impact category and specify "YES" or "NO". For each "YES," provide a concise description of the impact. For a release, this is especially crucial. Consider areas like API changes, configuration updates, or deprecated features.

  4. Provide testing details: List the specific host and target environments used for testing. Include relevant logs demonstrating the behavior before and after the change. This validates the functionality and helps reviewers assess the effectiveness of the testing process. Don't just rely on CI.

By addressing these points, the PR summary will provide the necessary information for reviewers to understand the changes and assess their impact effectively, ultimately leading to a smoother and more efficient review process.

@xiaoxiang781216 xiaoxiang781216 merged commit 52c4408 into apache:releases/12.8 Dec 11, 2024
@jerpelea jerpelea deleted the bp-15102 branch December 17, 2024 02:25
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.

4 participants