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-15353} fix compile error #15375

Merged
merged 1 commit into from
Dec 28, 2024
Merged

Conversation

jerpelea
Copy link
Contributor

Summary

chip/efm32_leserial.c: In function 'efm32_restoreuartint': Error: chip/efm32_leserial.c:329:37: error: 'len' undeclared (first use in this function); did you mean 'ien'?
329 | efm32_restoreuartint_nolock(priv, len);
| ^~~
| ien

Impact

RELEASE

Testing

CI

chip/efm32_leserial.c: In function 'efm32_restoreuartint':
Error: chip/efm32_leserial.c:329:37: error: 'len' undeclared (first use in this function); did you mean 'ien'?
  329 |   efm32_restoreuartint_nolock(priv, len);
      |                                     ^~~
      |                                     ien

Signed-off-by: hujun5 <[email protected]>
@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Size: S The size of the change in this PR is small labels Dec 28, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 28, 2024

[Experimental Bot, please feedback here]

No, this PR does not meet the NuttX requirements. The provided information only describes a build error, not the intended change. It's missing crucial information:

  • Missing context/purpose: What was the original intent of the change that introduced this error? A bug fix? A new feature? What problem is being solved?
  • Missing "how it works": The summary needs to describe the intended change, not just the error message. How should efm32_restoreuartint work? What was the logic behind the modification that led to the undeclared len variable?
  • Incomplete Impact Assessment: "RELEASE" is insufficient. While it suggests this impacts a release, it doesn't detail how. Which architectures or boards are affected? Does it change any user-facing behavior? Does it introduce any compatibility issues?
  • No Testing Information: "CI" is also inadequate. CI (Continuous Integration) is a process, not a result. The PR needs to show actual test results, logs demonstrating the issue before the change and the corrected behavior after the change. It should also specify the host build environment and the target platform. Providing build logs with the error doesn't count as testing the intended functionality.

The PR needs to be revised to provide the missing context, explain the intended change, and include proper testing logs demonstrating the fix. Just fixing the compiler error isn't enough; the PR must demonstrate that the fix achieves the desired outcome without introducing regressions.

@xiaoxiang781216 xiaoxiang781216 merged commit b187614 into apache:releases/12.8 Dec 28, 2024
24 of 25 checks passed
@jerpelea jerpelea deleted the bp-15353 branch December 29, 2024 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm Issues related to ARM (32-bit) architecture 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.

5 participants