[IMP] cli: rewrite help, fix bugs, modernize codebase#227
Open
[IMP] cli: rewrite help, fix bugs, modernize codebase#227
Conversation
Before this change, the CLI help output used terse, single-line descriptions with no examples, no default-value documentation, and no logical grouping. This made the tool difficult to discover and use without reading the source code. This commit rewrites all 19 argument help strings with: - Clear multi-line descriptions explaining each option's purpose - Consistent "(default: ...)" annotations on every argument - Logical grouping by category (Docker, Paths, Git, Build, Run, Hooks, DeployV) - A program-level description and usage examples in the epilog - RawDescriptionHelpFormatter to preserve formatting No logic changes — only cosmetic help text improvements.
Before this fix, the generated 20-run.sh contained duplicate -itP
flags: one hardcoded in the template and another injected via the
--run-extra-args CLI default ("-itP -e LANG=C.UTF-8"). This caused
Docker to receive "-itP -itP -e LANG=C.UTF-8" on every run, which
is harmless but confusing when debugging generated scripts.
The root cause was that the template hardcoded "-itP" independently
of the CLI default. Remove the hardcoded flags from the template so
the generated script only contains what --run-extra-args provides,
avoiding duplication and making the output predictable.
Before this fix, passing multiple variables in a single invocation (e.g. --build-env-args VAR1 VAR2) silently discarded all but the first value. The list comprehension used `x[0]` to extract only the first element of each sub-list produced by argparse's append action. This caused missing ARG/ENV lines in the generated Dockerfile, leading to broken builds when multiple build-time variables were needed. The fix changes the comprehension to properly flatten all sub-lists: `[arg for sublist in ... for arg in sublist]`.
The README documented a flat output structure (/1, /2) that did not match the actual code behavior. The code generates directories named by Python version and environment/job combination (e.g. 2_7/env_1_job_1). This confused users who couldn't find their generated scripts. This commit: - Correct the output path examples to match compute_dockerfile() - Document .t2d.yml as an alternative config file name (already supported by the code but never mentioned in docs) - Clarify that both SSH and HTTPS URLs are accepted - Recommend Ed25519 SSH keys (matching set_authorized_key() which checks id_ed25519.pub first) and keep RSA as a legacy fallback - Note that 10-build.sh uses --pull, so manual docker pull is optional
travis2docker no longer uses Travis CI for its own builds — the project migrated to GitHub Actions (.github/workflows/) which has been the active CI pipeline since the move to Vauxoo/travis2docker. The .travis.yml file was left behind and is misleading: it suggests the project still uses Travis CI, but builds have been running on GitHub Actions for years. Removing it avoids confusion for new contributors and eliminates a maintenance burden for dead config.
The codebase contained remnants from the Python 2 era that are
unreachable on Python 3:
- travis2docker.py: 5 try/except blocks calling .encode('utf-8')
before text-mode file.write(). On Python 3, str.encode() returns
bytes, which always raises TypeError on text-mode write — the
except branch was the only path ever executing. Remove the dead
try blocks entirely.
- setup.cfg: `universal = 1` declared the wheel as py2/py3
compatible, but the project requires Python >=3.6. Change to
`universal = 0` so bdist_wheel produces a py3-only wheel.
- git_run.py: remove `from __future__ import print_function`
(implicit in Python 3).
Additionally, update the default Python version from 3.5 to 3.10
in _python_version_env(). Python 3.5 reached end-of-life in 2020
and Odoo 16+ targets 3.10+, which is the primary use case for this
tool.
The run() method used bare print() calls to dump every git command and its argument list to stdout. This caused noisy output during normal usage, with no way to silence it short of redirecting stdout. Replace both print() calls with _logger.debug() so the git command tracing is only visible when the user explicitly enables debug-level logging (e.g. via logging.basicConfig(level=logging.DEBUG)). Also remove the now-unnecessary `print-used` pylint disable directive that was only needed because of these print statements.
Several CLI help strings were misleading or incomplete, causing users to waste time debugging unexpected behavior: - --docker-image: the default (vauxoo/odoo-80-image-shippable-auto) is a legacy Odoo 8.0 image that surprises users targeting modern Odoo versions. Add an explicit NOTE about this. - --runs-at-the-end-script: help said "(default: none)" but the code in Travis2Docker.__init__() actually injects "sleep 2" when the argument is omitted. This mismatch caused confusion when containers stayed alive unexpectedly. Fix the help to show the true default and explain how to disable it. - --build-env-args: in DeployV mode these variables are rendered as shell exports (not Docker ARG/ENV pairs), which is a non-obvious behavioral difference. Document this divergence. - --deployv: the auto-constructed image name pattern from variables.sh was undocumented. Add the naming pattern so users know what to expect.
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
1 participant
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
This PR improves the travis2docker CLI and codebase through 8 focused commits:
Why these changes?
CLI help was unusable — terse single-line descriptions with no examples, no defaults, and no grouping made the tool impossible to discover without reading source code.
Two bugs caused silent failures:
20-run.shhad duplicate-itPflags (hardcoded in template + CLI default)--build-env-args VAR1 VAR2silently discarded all but the first variable due tox[0]extractionREADME was misleading — documented a flat output structure (
/1,/2) that didn't match the actual versioned directory layout (2_7/env_1_job_1), and omitted.t2d.ymlsupport and Ed25519 SSH key recommendations.Dead CI config —
.travis.ymlwas left behind after migrating to GitHub Actions, confusing contributors.Python 2 vestiges — dead
try/except encode('utf-8')blocks,universal=1wheel config, and Python 3.5 as default version (EOL since 2020).Debug noise —
git_run.pyused bareprint()calls that polluted stdout with no way to silence them.Help strings had wrong defaults —
--runs-at-the-end-scriptdocumented(default: none)but code injected"sleep 2";--build-env-argsbehaves differently in DeployV mode (undocumented).Commits
[IMP][FIX]-itPflags in20-run.shtemplate[FIX]build_env_argscorrectly (was dropping values)[IMP].t2d.ymldocs[REM].travis.yml(using GitHub Actions)[REF][REF]print()withlogging.debug()[IMP]Supersedes
This PR supersedes the previous
main-study-cliPR with improved commit messages that include proper rationale following the project's commit conventions.