Skip to content

[CELEBORN-2396][CLI] Fix CelebornCli.main discarding the configured CommandLine instance - #3773

Closed
yew1eb wants to merge 3 commits into
apache:mainfrom
yew1eb:CELEBORN-2396
Closed

[CELEBORN-2396][CLI] Fix CelebornCli.main discarding the configured CommandLine instance#3773
yew1eb wants to merge 3 commits into
apache:mainfrom
yew1eb:CELEBORN-2396

Conversation

@yew1eb

@yew1eb yew1eb commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Call execute() on the configured CommandLine instance instead of creating and executing a new one.

Why are the changes needed?

CelebornCli.main creates a CommandLine, sets case-sensitivity options on it, but then executes a brand-new instance, so the configuration is silently discarded and the two setter calls are dead code.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

@SteNicholas SteNicholas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

SteNicholas pushed a commit that referenced this pull request Aug 3, 2026
…ommandLine instance

### What changes were proposed in this pull request?

Call `execute()` on the configured `CommandLine` instance instead of creating and executing a new one.

### Why are the changes needed?

`CelebornCli.main` creates a `CommandLine`, sets case-sensitivity options on it, but then executes a brand-new instance, so the configuration is silently discarded and the two setter calls are dead code.

### Does this PR introduce any user-facing change?

No.

### How was this patch tested?

Closes #3773 from yew1eb/CELEBORN-2396.

Authored-by: yew1eb <yew1eb@gmail.com>
Signed-off-by: Nicholas Jiang <programgeek@163.com>
(cherry picked from commit fd6aaa5)
Signed-off-by: Nicholas Jiang <programgeek@163.com>
@SteNicholas

Copy link
Copy Markdown
Member

Thanks. Merged to main(v1.0.0) and branch-0.7(v0.7.0).

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.

4 participants