Skip to content

Conversation

@aslonnie
Copy link
Collaborator

@aslonnie aslonnie commented Jan 7, 2026

Rename build_dir parameter to context_dir and move it to the last argument position for better API consistency.

🤖 Generated with Claude Code

@aslonnie aslonnie requested a review from a team as a code owner January 7, 2026 17:35
Rename build_dir parameter to context_dir and move it to the last
argument position for better API consistency.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Lonnie Liu <95255098+aslonnie@users.noreply.github.com>
@aslonnie aslonnie force-pushed the lonnie-260107-fillctxargs branch from a34eba8 to 00671a9 Compare January 7, 2026 17:36
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the fill_build_context_dir function by renaming the build_dir parameter to context_dir and adjusting the parameter order for better clarity. The changes are correctly propagated to the function's call sites in the application code and tests. My review found one minor issue: the function's docstring summary was not updated to reflect the parameter rename. Overall, this is a good change that improves code consistency.

Comment on lines 84 to 88
def fill_build_context_dir(
ctx: BuildContext,
build_dir: str,
source_dir: str,
context_dir: str,
) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Good refactoring on the parameter name for better clarity. To complete this change, could you also update the docstring summary on line 90? It still refers to build directory but should probably be context directory now to be fully consistent with the parameter rename.

@aslonnie aslonnie added alpha Alpha release features go add ONLY when ready to merge, run all tests and removed alpha Alpha release features labels Jan 7, 2026
@ray-gardener ray-gardener bot added core Issues that should be addressed in Ray Core release-test release test labels Jan 7, 2026

with tempfile.TemporaryDirectory() as build_dir:
fill_build_context_dir(build_context, build_dir, release_byod_dir)
fill_build_context_dir(build_context, release_byod_dir, build_dir)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this change is putting output at the end, at least not in the order of "input,output,input".

@aslonnie aslonnie merged commit a3b1033 into ray-project:master Jan 8, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Issues that should be addressed in Ray Core go add ONLY when ready to merge, run all tests release-test release test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants