Skip to content

Conversation

@tmedicci
Copy link
Contributor

Summary

After apache/nuttx-apps#2982, there is no need to mount the modules and manually set the required environment variables: they are all set by the new Python wrapper application.

Impact

Impact on user: YES. Users don't need to mount and set the environment variables anymore. Documentation should reflect it.

Impact on build: NO.

Impact on hardware: NO.

Impact on documentation: YES. Documentation is being fixed to reflect the current procedure to run Python.

Impact on security: NO.

Impact on compatibility: NO.

Testing

Just build the documentation locally:

cd Documentation
make html

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Size: XS The size of the change in this PR is very small labels Feb 20, 2025
@tmedicci tmedicci force-pushed the improvement/docs_python_wrapper branch from d49792e to 2585e16 Compare February 20, 2025 14:54
@github-actions github-actions bot added Size: S The size of the change in this PR is small and removed Size: XS The size of the change in this PR is very small labels Feb 20, 2025
@nuttxpr
Copy link

nuttxpr commented Feb 20, 2025

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements, although the testing section could be improved.

While the PR description clearly explains the changes and their impact, the testing section only mentions building the documentation. While documentation changes are part of the PR and should be tested, this doesn't demonstrate the core functionality change: the automated mounting and environment variable setting by the Python wrapper. The testing section should include logs demonstrating the Python application running successfully before the change (showing the manual steps) and after the change (showing the automated behavior). This provides concrete evidence that the change works as intended.

Copy link
Contributor

@hartmannathan hartmannathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor suggestion... Otherwise, LGTM. Thanks for helping to improve documentation!!

After apache/nuttx-apps#2982, there is no
need to mount the modules and manually set the required environment
variables: they are all set by the new Python wrapper application.

Signed-off-by: Tiago Medicci Serrano <tiago.medicci@espressif.com>
@tmedicci tmedicci force-pushed the improvement/docs_python_wrapper branch from 2585e16 to 3506e51 Compare February 20, 2025 15:20
@jerpelea jerpelea merged commit 798a73a into apache:master Feb 21, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Documentation Improvements or additions to documentation 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