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

cmake(bugfix):add INCLUDE_DIRECTORIES for nimble #2846

Merged
merged 1 commit into from
Nov 13, 2024

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 <[email protected]>
@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