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-13569} Revert "toolchain/ghs: Fix CONFIG_SCHED_CRITMONITOR_MAXTIME_XXX "zero… #13974

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

jerpelea
Copy link
Contributor

@jerpelea jerpelea commented Oct 9, 2024

Summary

… used for undefined preprocessing identifier" warnings"

move private define to public

This reverts commit 236678d.

Impact

RELEASE

Testing

CI

… used for undefined preprocessing identifier" warnings"

move private define to public

This reverts commit 236678d.

Signed-off-by: chao an <anchao@lixiang.com>
@github-actions github-actions bot added Area: OS Components OS Components issues Size: S The size of the change in this PR is small labels Oct 9, 2024
@jerpelea jerpelea changed the title Revert "toolchain/ghs: Fix CONFIG_SCHED_CRITMONITOR_MAXTIME_XXX "zero… {Bp-13569} Revert "toolchain/ghs: Fix CONFIG_SCHED_CRITMONITOR_MAXTIME_XXX "zero… Oct 9, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 9, 2024

[Experimental Bot, please feedback here]

No, this PR does not meet the NuttX requirements.

Missing Information:

  • Summary:
    • Lacks a clear explanation of why the change is necessary. What were the problems caused by the original commit (236678d)?
    • What specific "undefined preprocessing identifier warnings" were encountered?
    • Which define is being moved from private to public and why?
  • Impact:
    • Simply stating "RELEASE" is not sufficient. Explain the potential impact of this change on users, the build process, hardware, documentation, security, and compatibility. Provide details about:
      • Will users need to modify their code or configurations?
      • Will any build procedures be affected?
      • Are there implications for specific hardware architectures or boards?
      • Does this change introduce any security vulnerabilities or address existing ones?
      • Will this change break compatibility with previous versions of NuttX?
  • Testing:
    • "CI" is not enough. Provide specific details about your local testing environment:
      • Host OS and version
      • Compiler and version
      • Target architecture and board configuration
    • Include relevant snippets of testing logs from both before and after the change, clearly demonstrating the issue and the fix.

Recommendations:

  1. Expand the Summary: Provide a thorough explanation of the problem, the proposed solution, and the rationale behind it.
  2. Detail the Impact: Analyze and describe all potential consequences of this change, especially if it could affect users or introduce compatibility issues.
  3. Provide Specific Testing Information: Clearly outline your testing environment and include relevant log snippets that highlight the problem and the effectiveness of your fix.

A well-documented PR is crucial for maintainers to understand and review your changes effectively.

@jerpelea
Copy link
Contributor Author

jerpelea commented Oct 9, 2024

apache/nuttx-apps#2661 needed

@xiaoxiang781216 xiaoxiang781216 merged commit 9dbde04 into apache:releases/12.7 Oct 9, 2024
10 of 29 checks passed
@jerpelea jerpelea deleted the bp-13569 branch October 9, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OS Components OS Components issues 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