Skip to content

interpreters/luamodules/luv: use "depends on" instead of "select"#2902

Merged
xiaoxiang781216 merged 1 commit intoapache:masterfrom
yamt:lua-luv
Dec 18, 2024
Merged

interpreters/luamodules/luv: use "depends on" instead of "select"#2902
xiaoxiang781216 merged 1 commit intoapache:masterfrom
yamt:lua-luv

Conversation

@yamt
Copy link
Contributor

@yamt yamt commented Dec 18, 2024

Summary

interpreters/luamodules/luv: use "depends on" instead of "select"

Impact

Testing

build-tested with apache/nuttx#15264

@nuttxpr
Copy link

nuttxpr commented Dec 18, 2024

[Experimental Bot, please feedback here]

Fill In The Commit Message: This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary.

No, this PR does not fully meet the NuttX requirements. While it provides a summary of the change itself, it lacks crucial information.

Specifically, it's missing:

  • Detailed explanation of why the change from "select" to "depends on" is necessary. What problem does this solve? What are the benefits?
  • Description of the functional part of the code being changed. Is this a build system change? A runtime change?
  • How the change works. A more technical description than just the title is needed. How does using "depends on" instead of "select" affect the build process or runtime behavior?
  • Issue references. Are there any related issues in the NuttX or NuttX Apps repositories?
  • Impact assessment is entirely empty. This section needs to be filled out. At minimum, justify the "NO" answers, or explain the "YES" answers. Consider build impact, hardware impact, documentation needs, etc.
  • Testing is insufficient. "build-tested" is vague. What was built? What target? While referencing another PR might be relevant, it doesn't replace providing actual testing logs or, at the very least, a clear description of the test procedure and results. Before/after logs are expected whenever possible.

In short, the PR needs significantly more detail to meet the requirements. It should clearly explain the motivation, implementation, and impact of the change, along with appropriate testing verification.

@xiaoxiang781216 xiaoxiang781216 merged commit d8faa9b into apache:master Dec 18, 2024
13 of 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