Skip to content

IGNITE-24814 Allow overriding variables in start script#7777

Merged
PakhomovAlexander merged 2 commits intoapache:mainfrom
unisonteam:IGNITE-24814
Apr 3, 2026
Merged

IGNITE-24814 Allow overriding variables in start script#7777
PakhomovAlexander merged 2 commits intoapache:mainfrom
unisonteam:IGNITE-24814

Conversation

@valepakh
Copy link
Copy Markdown
Contributor

@PakhomovAlexander
Copy link
Copy Markdown
Contributor

Code Review

Overall this is a solid, well-structured change. Two suggestions worth addressing:

1. Unknown --* arguments are silently swallowed

In the Linux parse_args, any unrecognized argument (e.g., a typo like --nde-name) is silently shifted away without any feedback. This makes misconfiguration hard to debug. Consider emitting a warning for unrecognized --* flags:

--*)
    echo "Warning: unknown option '$1'" >&2
    ;;

Same applies to the Windows :parseArgs — unrecognized --* args silently fall through to shift.

2. No --help flag

The script now accepts 5 new CLI flags (--node-name, --work-dir, --log-dir, --config, --extra-classpath) but provides no usage message. A --help / -h case in the parser printing available options would be valuable for discoverability, especially since these flags don't appear to be documented anywhere else in the PR.

Copy link
Copy Markdown
Contributor

@PakhomovAlexander PakhomovAlexander left a comment

Choose a reason for hiding this comment

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

see comment

@valepakh
Copy link
Copy Markdown
Contributor Author

valepakh commented Apr 2, 2026

Done

@PakhomovAlexander PakhomovAlexander merged commit 437a40e into apache:main Apr 3, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants