Repository navigation
[None][fix] Correctly select zsh in devcontainer - #21384
Conversation
- Set the SHELL environment variable to zsh in the devcontainer. This was already configured in the devcontainer features but did not work as expected since the container was still inherited the default shell. - Format the devcontainer configuration. Signed-off-by: Robin Kobus <19427718+Funatiq@users.noreply.github.com>
…idance Signed-off-by: Robin Kobus <19427718+Funatiq@users.noreply.github.com>
…ntegration Signed-off-by: Robin Kobus <19427718+Funatiq@users.noreply.github.com>
|
/bot skip --comment "only change devcontainer config" |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Shell and history startup configuration .devcontainer/devcontainer.json |
The container environment adds SHELL=/bin/zsh. A startup command creates .cache and writes the HISTFILE setting to an Oh My Zsh custom configuration file. The VS Code settings and container features remain unchanged. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers: bowenfu
Merge Risk: 
6c772
-
The devcontainer history setup is correctly configured and presents no actionable merge risk.
Pre-merge checks: 
-
Check name Status Explanation Resolution Description check The description explains the three commits: setting SHELL to /bin/zsh, adding a startup hook to restore HISTFILE through Oh My Zsh, and updating documentation links and indentation. It leaves the temp… 1. Fill in the Test Coverage section. List the tests or manual checks that verify the devcontainer change, or state that no automated test applies and explain how the change was checked.
2. Correct the history file path. The description say…-
Check name Status Explanation Title check The title "[None][fix] Correctly select zsh in devcontainer" follows the required template with a ticket placeholder and a lowercase type. It matches the main change, which sets SHELL to /bin/zsh in d… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "[None][fix] Correctly select zsh in devcontainer" follows the required template with a ticket placeholder and a lowercase type. It matches the main change, which sets SHELL to /bin/zsh in devcontainer.json. It does not mention the HISTFILE restore hook or the doc link updates, which is acceptable for a title.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
-
Full details: Description check
-
Explanation
The description explains the three commits: setting SHELL to /bin/zsh, adding a startup hook to restore HISTFILE through Oh My Zsh, and updating documentation links and indentation. It leaves the template's Test Coverage section empty, with only the HTML comment prompt left in place. The Description section also keeps the template comment, and the description does not state the reason for the change in full. The PR checklist is marked complete. The description states the history file as .cache/._bash_history, but the commit messages and objectives describe .cache/._zsh_history for zsh, so the description conflicts with them. The author should fix that conflict and fill in the Test Coverage section.
Resolution
- Fill in the Test Coverage section. List the tests or manual checks that verify the devcontainer change, or state that no automated test applies and explain how the change was checked.
- Correct the history file path. The description says
.cache/._bash_history, but the commit messages say.cache/._zsh_historyfor zsh. Confirm the correct path and make the description match it. - Expand the Description section with the reason for the change. Explain why VS Code overrides HISTFILE and why the restore hook is needed.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
|
PR_Github #81836 [ skip ] triggered by Bot. Commit: |
|
PR_Github #81836 [ skip ] completed with state |
brnguyen2
left a comment
brnguyen2
left a comment
There was a problem hiding this comment.
Approving — the comments below are optional touch-ups, not blockers.
The re-indent plus the two functional lines make the diff look much bigger than it is. Reviewers can use git diff -w. In future, putting the formatting change in its own commit makes this easier.
There's no test coverage, so please add the manual check to the PR's Test Coverage section: rebuild the container, open a new VS Code terminal, run echo $SHELL $HISTFILE, and confirm that history survives a container restart.
|
Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements. |
Configure the Oh My Zsh startup override to use .cache/._zsh_history while retaining .cache/._bash_history for bash. This keeps zsh extended history entries separate from bash history while preserving both files in the workspace mount. Signed-off-by: Robin Kobus <19427718+Funatiq@users.noreply.github.com>
|
/bot skip --comment "only change devcontainer config" |
|
PR_Github #82031 [ skip ] triggered by Bot. Commit: |
|
PR_Github #82031 [ skip ] completed with state |
Description
The PR contains three commits affecting
devcontainer.json:SHELL=/bin/zsh.HISTFILEthrough Oh My Zsh, keeping history in the host-mounted.cache/._bash_historydespite VS Code’s override.Dev Engineer Review
The devcontainer sets
SHELLto/bin/zshand enables zsh as the default shell. ItspostStartCommandcreates.cacheand writes an Oh My Zsh startup hook that setsHISTFILEto.cache/._zsh_history. The configured container environment still setsHISTFILEto.cache/._bash_history; zsh uses the custom startup hook to override that path.QA Engineer Review
No test changes.
Per-File QA Perspective
.devcontainer/devcontainer.json: Verify that a new devcontainer session starts with zsh and that zsh history persists at.cache/._zsh_history. The configuration also retains.cache/._bash_historyas the container-levelHISTFILEvalue.Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.