Skip to content

CLI improvements#16

Merged
MatDagommer merged 2 commits intomainfrom
cli-improvements
Jan 5, 2026
Merged

CLI improvements#16
MatDagommer merged 2 commits intomainfrom
cli-improvements

Conversation

@MatDagommer
Copy link
Owner

  • Corrected templates/.gitignore
  • Allow calling pyml with no arguments in terminal (returns help)

- Set no_args_is_help=True in Typer app initialization to show help message when CLI is run without arguments.
…nd move Dockerfile to .devcontainer directory

- Replaced custom .gitignore with a comprehensive Python .gitignore template for better coverage of common files to ignore.
- Moved Dockerfile into the .devcontainer directory to align with devcontainer standards.
@MatDagommer MatDagommer merged commit c95d44a into main Jan 5, 2026
4 of 5 checks passed
@MatDagommer MatDagommer deleted the cli-improvements branch January 5, 2026 18:15
@MatDagommer MatDagommer restored the cli-improvements branch January 5, 2026 18:15
@github-actions
Copy link

github-actions bot commented Jan 5, 2026

PR Preview Action v1.6.3

🚀 View preview at
https://MatDagommer.github.io/pyml-cli/pr-preview/pr-16/

Built to branch gh-pages at 2026-01-05 18:15 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@claude
Copy link

claude bot commented Jan 5, 2026

Code Review: CLI improvements

Summary

This PR makes two main changes:

  1. Adds no_args_is_help=True to the Typer CLI app
  2. Corrects the .gitignore template (replacing bump2version config with proper Python gitignore patterns)
  3. Moves Dockerfile to .devcontainer/Dockerfile in the project template

Positive Findings

Code Quality

  • Clean implementation: The no_args_is_help=True addition is well-placed and follows Typer best practices
  • Proper formatting: The multi-line Typer initialization improves readability (pyml_cli/cli/init.py:12-15)
  • Standards-based gitignore: Uses the comprehensive Python gitignore from toptal.com/developers/gitignore

User Experience Improvement

  • Better UX: Running pyml without arguments will now show help instead of doing nothing, which aligns with standard CLI conventions
  • Proper project structure: Moving Dockerfile to .devcontainer/ follows VS Code dev container conventions

🔍 Issues & Concerns

Critical: .gitignore File Corruption

The diff shows that the original .gitignore file contained bump2version configuration in TOML format. This indicates the file was likely corrupted or incorrectly created in the first place - gitignore files should never contain TOML configuration.

Questions:

  1. Was this .gitignore file always corrupted, or was it accidentally overwritten with .bumpversion.toml content?
  2. Should there be a separate .bumpversion.toml or similar file in the template that was lost?
  3. Do you need to preserve any bump2version configuration elsewhere?

Recommendation: Verify that no bump2version configuration was lost. If it's needed, it should go in pyproject.toml under [tool.bumpversion] or in a separate .bumpversion.toml file.

Missing Test Coverage

While tests/test__init__.py exists with a test for the configure command, there's no test for the new no_args_is_help behavior.

Recommendation: Add a test case:

def test_no_args_shows_help():
    """Test that running pyml with no arguments shows help."""
    result = runner.invoke(app, [])
    assert result.exit_code == 0
    assert "Usage:" in result.stdout or "Commands:" in result.stdout

Dockerfile Move

The diff shows Dockerfile moved to .devcontainer/Dockerfile, but:

  • No .devcontainer/devcontainer.json is visible in the diff
  • Are there any references to Dockerfile in other template files that need updating?
  • Should this be documented as a breaking change for existing projects?

🔒 Security Considerations

Positive:

  • The new .gitignore properly excludes sensitive files (.env, credentials, private keys)
  • Includes common security-sensitive patterns like *.log, *.pot, database files

Note: The custom additions at the end include .DS_Store and docs/cli.md - ensure docs/cli.md exclusion is intentional (it might contain auto-generated CLI documentation that should be ignored).


Performance Considerations

No performance concerns. The no_args_is_help=True parameter adds negligible overhead.


📝 Best Practices & Suggestions

  1. Documentation: Consider updating the README or docs to mention the Dockerfile location change if projects depend on it

  2. Changelog: This PR should be documented in release notes as it:

    • Changes user-facing CLI behavior (help on no args)
    • Changes project template structure (Dockerfile location)
    • Fixes a corrupted template file
  3. Template Validation: Consider adding tests that validate template file integrity, especially for critical files like .gitignore

  4. Consistency Check: Verify that cookiecutter.json or other template logic doesn't reference the old Dockerfile path


📊 Test Coverage Assessment

Current coverage: Partial - only the configure command is tested
Recommended additions:

  • Test for no_args_is_help behavior
  • Integration test that validates the generated project template structure
  • Test that verifies .gitignore patterns work as expected

Approval Recommendation

Approve with minor suggestions - The core changes are solid, but please:

  1. Verify no bump2version config was lost
  2. Add test for no_args_is_help behavior
  3. Confirm .devcontainer/Dockerfile move won't break existing workflows
  4. Document the template changes in release notes

📋 Checklist for Merge

  • Confirm bump2version configuration isn't needed or moved to proper location
  • Add test case for no_args_is_help behavior
  • Verify no references to old Dockerfile path in templates
  • Update documentation if needed
  • Add changelog entry for template structure changes

Great work on improving the CLI UX! The no_args_is_help addition is a nice quality-of-life improvement. 👍

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.

1 participant