Skip to content

Conversation

@xuxin930
Copy link
Contributor

Summary

Enable various types of common applications and libraries, and enable CMake CI checks.
To avoid regression issues in the build system.
such as: #14780

Impact

add CI test coverage configuration

Testing

# build
cmake -B build -DBOARD_CONFIG=qemu-armv7a:full -GNinja
cmake --build build

# run
qemu-system-arm -cpu cortex-a7 -nographic \    
     -machine virt,virtualization=off,gic-version=2 \
     -net none -chardev stdio,id=con,mux=on -serial chardev:con \
     -mon chardev=con,mode=readline -kernel ./build/nuttx -s

@github-actions github-actions bot added Area: Tooling Area: CI Board: arm Size: M The size of the change in this PR is medium labels Nov 14, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 14, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. Here's why:

  • Summary: While it mentions the "why" (avoiding regressions), the "what" is too vague. "Various types of common applications and libraries" needs to be far more specific. List the actual applications and libraries enabled. The "how" is also missing – how does enabling CMake CI checks prevent regressions? Explain the mechanism.
  • Impact: "add CI test coverage configuration" is insufficient. Address all impact points individually (user, build, hardware, documentation, security, compatibility). Even if the answer is "NO", state it explicitly. For the build impact, specifying "add CI test coverage configuration" is still vague. How does this affect the build process for regular users, not just CI?
  • Testing: The provided testing logs show how to build and run NuttX. They do not demonstrate the actual change introduced by the PR or how the new functionality has been verified. Show logs relevant to the specific applications and libraries enabled and the CMake CI checks. Demonstrate the tests passing before and after the change. Also, specifying the host build environment is missing.

In short: The PR description lacks crucial details about the changes, their impact, and verification. It needs to be significantly more specific and demonstrate the changes' effectiveness.

enable various types of common applications and libraries, and enable CMake CI checks.
To avoid regression issues in the build system

Signed-off-by: xuxin19 <xuxin19@xiaomi.com>
@xiaoxiang781216 xiaoxiang781216 merged commit d2a7e45 into apache:master Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: CI Area: Tooling Board: arm Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants