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

{bp-15120} libc/chdir: chdir/fchdir should not depend on environment variables #15134

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

jerpelea
Copy link
Contributor

Summary

This PR will still allow basic shell operations such as cd/ls/pwd to be used even when the environment is disabled.

Impact

RELEASE

Testing

CI

This PR will still allow basic shell operations such as cd/ls/pwd to be used even when the environment is disabled.

Signed-off-by: chao an <[email protected]>
@github-actions github-actions bot added Area: OS Components OS Components issues Size: S The size of the change in this PR is small labels Dec 11, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 11, 2024

[Experimental Bot, please feedback here]

No, this PR summary does not meet the NuttX requirements. It's missing crucial information.

Here's why:

  • Insufficient Detail in Summary: While it states the what, it lacks the why and how. Why is this change necessary? Was there a bug? Is it a performance improvement? How does it achieve this functionality? What code was modified?
  • Missing Issue References: Are there related issues in either the NuttX or NuttX-apps repositories? These should be linked.
  • Incomplete Impact Assessment: "RELEASE" is not sufficient. Each impact category needs a YES/NO answer and a description if YES. For example, even if the user impact is minimal, you still need to state "Impact on user: NO". Consider the impact on the build system, hardware, documentation, security, and compatibility.
  • Insufficient Testing Information: "CI" is not enough. While CI testing is important, you need to provide specifics about your local testing. What host did you build on? What target hardware or simulator did you test on? The template specifically requests OS, CPU, compiler, architecture, board, and configuration details. Critically, the "before" and "after" log sections are empty. You must demonstrate that the change works and include evidence.

Example of a better Summary:

This PR fixes a regression introduced in [commit hash or issue #] where basic shell commands (cd, ls, pwd) were unavailable when the user environment was disabled. This change modifies the shell initialization logic in apps/examples/nsh/nsh_main.c to ensure these commands are always registered, regardless of environment settings. This resolves the issue by directly calling the built-in command handlers instead of relying on environment lookups.

This would then be followed by properly filled-out Impact and Testing sections.

@xiaoxiang781216 xiaoxiang781216 merged commit 32ab151 into apache:releases/12.8 Dec 11, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OS Components OS Components issues 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.

4 participants