[IMP] cli: rewrite help, fix bugs, modernize codebase - #227
Closed
nhomar wants to merge 5 commits into
Closed
Conversation
Using --build-env-args with multiple values in a single flag, e.g.:
--build-env-args VIM_INSTALL ZSH_INSTALL
silently discarded every value but the first one, because the parsing
only extracted item[0] from each appended nargs list. This is exactly
the usage documented in the README, so it was broken as documented.
Flatten the nested lists instead, so both the repeated-flag form and
the multi-value form generate every ENV line in the Dockerfile.
Every git command executed was printed twice to stdout with bare print() calls, polluting the tool output with debug noise that could not be silenced. Use logging.debug() instead, so it stays hidden by default and can be enabled when actually debugging.
The 20-run.sh template already hardcodes -ditP in the docker run command, so the -itP included in the --run-extra-args default value produced 'docker run -itP ... -ditP ...' with every flag repeated. Docker tolerates the repetition, but it is confusing when reading the generated script and the --help default. Keep only the LANG export in the default value.
The --help output had terse descriptions with no examples nor default values, making the tool hard to discover without reading the source. - Add a program description explaining what the tool actually does today: generate a Dockerfile and helper scripts from the deployv image of a repository based on its variables.sh file. - Add an epilog with usage examples and the TRAVIS2DOCKER_ROOT_PATH environment variable. - Document the default value of every parameter in its help text. - Fix the --build-env-args help: it documented 'ARG NAME' and 'ENV NAME=$NAME' lines, but the deployv template actually generates 'ENV NAME=TRUE' lines used to enable optional installation steps such as VIM_INSTALL and ZSH_INSTALL. - Clarify that -ditP is already hardcoded in 20-run.sh so it does not need to be passed via --run-extra-args.
The tool prefers ~/.ssh/id_ed25519.pub for the container's authorized_keys and warns that RSA keys are deprecated, but the README only documented how to remove the passphrase from RSA keys. Document the Ed25519 flow first and keep the RSA one as legacy. Also clarify that https urls are supported for public repositories and remove trailing whitespace.
moylop260
force-pushed
the
main-study-cli-v2
branch
from
August 7, 2026 22:12
8493fbc to
fd61d0f
Compare
Collaborator
|
Superseded by #238 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale
Rebased on
main(7.0.0) keeping only the changes that still apply after the deployv-only refactor (.travis.ymlsupport was dropped in the meantime, the tool now works exclusively from thevariables.shof the deployv image).Changes kept (adapted to the current codebase)
[FIX]--build-env-args VAR1 VAR2silently discarded all but the first variable due tox[0]extraction (this is exactly the usage documented in the README). Flatten instead, with test coverage.[REF]git_run.pyused bareprint()calls that polluted stdout with no way to silence them; uselogging.debug().[FIX]--run-extra-argsdefault-itPduplicated the-ditPalready hardcoded in the20-run.shtemplate.[IMP]--helpdescriptions: program description/epilog with examples, document every default, and fix the wrong--build-env-argshelp (the deployv template generatesENV NAME=TRUE, notARG/ENV NAME=$NAME).[IMP]id_ed25519.puband warns RSA is deprecated), keep RSA as legacy.Changes dropped (already obsolete on
main).travis.yml— already removed onmain.setup.cfguniversal = 0— the[bdist_wheel]section no longer exists.encode('utf-8')try/except) — already removed by the pathlib migration / pre-commit autofixes._python_version_envno longer exists (deployv-only mode).20-run.shrewrite — superseded by the detach-aware rewrite onmain(-ditP+docker attach+ ownership fix).2_7/env_1_job_1) — the output structure is flat now; the README onmainis already correct..t2d.ymldocs and help texts referencing.travis.ymlparsing — that mode no longer exists; help texts were rewritten for thevariables.sh/deployv reality instead.