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

[bp-15102} update documentation #15133

Merged
merged 3 commits into from
Dec 11, 2024
Merged

Conversation

jerpelea
Copy link
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 <[email protected]>
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.
@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
Copy link

nuttxpr commented Dec 11, 2024

[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
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.

4 participants