Skip to content

Conversation

@xuxin930
Copy link
Contributor

Summary

/github/workspace/sources/apps/examples/nimble/nimble_main.c:40:10: fatal error: nimble/nimble_npl.h: No such file or directory
40 | #include "nimble/nimble_npl.h"
| ^~~~~~~~~~~~~~~~~~~~~
compilation terminated.
ninja: build stopped: subcommand failed.

Impact

bugfix

Testing

cmake -B build -DBOARD_CONFIG=nrf52840-dk/sdc_nimble -GNinja

build pass

/github/workspace/sources/apps/examples/nimble/nimble_main.c:40:10: fatal error: nimble/nimble_npl.h: No such file or directory
   40 | #include "nimble/nimble_npl.h"
      |          ^~~~~~~~~~~~~~~~~~~~~
compilation terminated.
ninja: build stopped: subcommand failed.

Signed-off-by: xuxin19 <xuxin19@xiaomi.com>
@nuttxpr
Copy link

nuttxpr commented Nov 13, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it identifies a build issue and appears to fix it, it severely lacks critical information. Here's why:

  • Insufficient Summary: The summary only shows a compiler error. It doesn't explain why the error occurred, what change was made to fix it, how the fix works, or if any related NuttX issues exist. What was the root cause of the missing header? Was a path incorrect? Was a dependency missing? Was the header added? How?
  • Incomplete Impact Assessment: While "bugfix" is a start, it's too vague. Which architectures/boards does this fix affect? Are there any implications for users (do they need to rebuild anything)? Does it change anything in the build system itself? Documentation?
  • Missing Testing Details: It mentions a build command but provides no actual logs. We need to see build output before the change showing the failure and after the change demonstrating the successful build. "build pass" is not sufficient evidence. Crucially, it also lacks details about the host build environment.

In short, the PR needs to provide a clear explanation of the problem, the solution, and verifiable evidence that the solution works. Simply stating there was a compile error and it's now fixed is not enough information for proper review and integration.

@xiaoxiang781216 xiaoxiang781216 merged commit 1c7a7f7 into apache:master Nov 13, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants