Skip to content

Conversation

tony
Copy link
Member

@tony tony commented May 10, 2025

Fixes:

Summary by Sourcery

Add new CLI commands add and add-from-fs to vcspull for managing repository configuration

New Features:

  • Implement add command to add a single repository to vcspull configuration
  • Implement add-from-fs command to scan and add multiple git repositories from a directory

Enhancements:

  • Extend CLI parser to support new subcommands
  • Improve configuration file handling with more flexible path and key management

Copy link

sourcery-ai bot commented May 10, 2025

Reviewer's Guide

This pull request introduces two new CLI subcommands, add and add-from-fs, to vcspull, enhancing its repository management capabilities. The add command is implemented in src/vcspull/cli/add.py and allows users to manually specify repository details (URL, name, target path) for addition to the YAML configuration. The add-from-fs command, housed in src/vcspull/cli/add_from_fs.py, scans a specified filesystem directory for Git repositories, automatically extracts their remote 'origin' URLs, and adds them to the configuration, optionally after user confirmation. Both functionalities are integrated into the main CLI by updating src/vcspull/cli/__init__.py to include new argument subparsers and modify the command dispatch logic. This involved refactoring create_parser to manage and return a collection of all subparsers and updating the cli function to robustly pass arguments to the respective command handlers.

File-Level Changes

Change Details Files
Implemented add command for manual repository addition to the configuration.
  • Created add.py module with logic to parse repository details (URL, name, path) and update the YAML configuration.
  • Defined command-line arguments for the add subparser within add.py.
  • Integrated the add subparser and its handler into the main CLI structure in __init__.py.
src/vcspull/cli/add.py
src/vcspull/cli/__init__.py
Implemented add-from-fs command to discover local Git repositories and add them to the configuration.
  • Created add_from_fs.py module with logic to scan directories, extract Git remote URLs, and update the configuration, including an interactive confirmation step.
  • Defined command-line arguments for the add-from-fs subparser within add_from_fs.py.
  • Integrated the add-from-fs subparser and its handler into the main CLI structure in __init__.py.
src/vcspull/cli/add_from_fs.py
src/vcspull/cli/__init__.py
Refactored CLI argument parsing and command dispatching in __init__.py.
  • Modified create_parser function to support the registration and retrieval of multiple subparsers.
  • Updated the main cli function to correctly dispatch to the new command handlers based on the invoked subparser.
  • Improved argument handling when calling command handlers by checking for attribute existence on the parsed arguments object.
src/vcspull/cli/__init__.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@tony tony changed the title !squash initial: add and add-from-fs New commands: add and add-from-fs May 10, 2025
@tony tony force-pushed the scanner-and-add branch 2 times, most recently from a36fbef to 69a222e Compare May 10, 2025 16:36
Copy link

codecov bot commented May 10, 2025

Codecov Report

❌ Patch coverage is 72.62873% with 101 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.01%. Comparing base (1570e0c) to head (9c5c92e).

Files with missing lines Patch % Lines
src/vcspull/cli/add_from_fs.py 71.42% 28 Missing and 10 partials ⚠️
src/vcspull/cli/fmt.py 74.80% 20 Missing and 13 partials ⚠️
src/vcspull/cli/add.py 61.64% 22 Missing and 6 partials ⚠️
src/vcspull/cli/sync.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #465      +/-   ##
==========================================
- Coverage   78.93%   77.01%   -1.93%     
==========================================
  Files           8       11       +3     
  Lines         413      770     +357     
  Branches       85      174      +89     
==========================================
+ Hits          326      593     +267     
- Misses         52      114      +62     
- Partials       35       63      +28     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tony tony force-pushed the scanner-and-add branch 3 times, most recently from d37cb7e to 628830e Compare May 17, 2025 11:49
@tony tony force-pushed the scanner-and-add branch 3 times, most recently from e85482d to e26036d Compare June 1, 2025 11:07
@tony tony force-pushed the scanner-and-add branch 6 times, most recently from 740c98c to 1045626 Compare June 22, 2025 14:09
@tony tony force-pushed the scanner-and-add branch from ca15fce to c68f1d5 Compare June 28, 2025 11:57
@tony tony force-pushed the scanner-and-add branch from c68f1d5 to 32bbbd8 Compare July 5, 2025 12:09
@tony tony force-pushed the scanner-and-add branch from 1a6b722 to 40c47a6 Compare July 12, 2025 14:02
@tony tony force-pushed the scanner-and-add branch from 40c47a6 to 1e1becb Compare July 20, 2025 21:06
@tony tony force-pushed the scanner-and-add branch 9 times, most recently from b02d442 to 27d3297 Compare September 4, 2025 11:08
tony added 27 commits September 5, 2025 10:14
why: Need consistent YAML writing functionality for new CLI commands
what:
- Add save_config_yaml function to centralize config file writing
- Use ConfigReader._dump for consistent formatting

refs: Foundation for add/add-from-fs commands
why: Users need a way to manually add individual repositories to config
what:
- Add command to add single repository by name and URL
- Support custom base directory with --dir option
- Support path inference with --path option
- Check for duplicates and warn appropriately
- Use verbose format {"repo": "url"} for new entries
…ition

why: Users with many existing repos need efficient bulk import
what:
- Scan directories for git repositories
- Extract origin URLs from git config
- Support recursive scanning with -r/--recursive
- Custom base directory key with --base-dir-key
- Auto-confirm with -y/--yes
- Show summary for large numbers of existing repos
why: Sync function needs to handle cases where config is not specified
what:
- Change sync function signature to accept optional config
- Update CLI handler to properly pass config parameter
why: CLI commands need user-friendly output without log prefixes
what:
- Add SimpleLogFormatter class for message-only output
- Configure CLI modules (add, add_from_fs, sync) to use simple formatter
- Maintain debug formatter for core vcspull logger
- Set propagate=False to prevent duplicate logging
why: Ensure add command functionality is properly tested
what:
- Test simple repository addition
- Test custom base directory handling
- Test duplicate detection
- Test adding to existing config
…canner

why: Ensure add-from-fs functionality works correctly
what:
- Test git origin URL extraction
- Test single and multiple repo discovery
- Test recursive and non-recursive scanning
- Test user confirmation flow
- Test existing repo handling
- Test output formatting for many repos
why: Ensure logging infrastructure works as expected
what:
- Test SimpleLogFormatter output
- Test RepoLogFormatter functionality
- Test DebugLogFormatter with various log levels
- Test logger configuration and propagation
why: Some CLI output goes to stderr and was being missed
what:
- Capture both stdout and stderr in test_sync_cli_filter_non_existent
- Ensures complete output validation in test
why: Tests didn't follow established vcspull testing patterns
what:
- Convert to parameterized tests using AddRepoFixture(NamedTuple)
- Test through CLI entry point instead of direct function calls
- Add clear_logging_handlers fixture to prevent stream closure issues
- Remove caplog.set_level() calls that were causing handler conflicts
- Maintain full test coverage with improved organization

refs: #tests
…entions

why: Tests didn't follow established vcspull testing patterns and had logging issues
what:
- Convert to parameterized tests using AddFromFsFixture(NamedTuple)
- Test through CLI entry point for integration testing
- Add helper functions setup_git_repo() and clone_repo() for test setup
- Add clear_logging_handlers fixture to prevent stream closure issues
- Remove caplog.set_level() calls that were causing handler conflicts
- Improve test organization and maintain full coverage

refs: #tests
…ception handler

why: Match established error handling patterns in the codebase
what:
- Change raise to return in save_config_yaml exception handler
- Maintains consistency with other modules' error handling approach
…se in exception handler

why: Match established error handling patterns in the codebase
what:
- Change raise to return in save_config_yaml exception handler
- Maintains consistency with other modules' error handling approach
why: Address ruff linting warnings to improve code quality and consistency
what:
- Move traceback imports to module level instead of conditional imports
- Convert f-string logging to % formatting for better performance
- Fix line length issues to meet 88 character limit
- Maintain all functionality while improving code style

All tests passing after changes.
why: Follow imperative mood convention for docstrings (D401)
what:
- Change "Helper to set up" to "Set up" in setup_git_repo docstring
- Change "Helper to clone" to "Clone" in clone_repo docstring

refs: D401 ruff rule
why: Users need a way to standardize and clean up their vcspull configurations
what:
- Add format_config_file function to format vcspull configs
- Implement normalize_repo_config to convert compact to verbose format
- Convert url keys to repo keys for consistency
- Sort directories and repositories alphabetically
- Add --write flag requirement for actual file modification
- Show detailed summary of changes when run without --write
why: Wire up the fmt command to the main CLI interface
what:
- Import fmt module and format_config_file function
- Add fmt_parser to subparsers with help text
- Add fmt handling in cli() function with config and write arguments
- Update create_parser return tuple to include fmt_parser
why: Ensure fmt command works correctly with various config formats
what:
- Add TestNormalizeRepoConfig class for testing repo normalization
- Add TestFormatConfig class for testing config formatting logic
- Add TestFormatConfigFile class for testing file operations
- Test compact to verbose format conversion
- Test url to repo key conversion
- Test directory and repository sorting
- Test --write flag behavior and dry-run mode
- Add clear_logging_handlers fixture to prevent test interference
why: mypy reported type errors when unpacking kwargs to format_config_file
what:
- Remove intermediate fmt_kwargs dictionary
- Pass arguments directly to format_config_file in correct order
- Fixes incompatible type errors for both arguments

refs: mypy error: "Argument 1 to format_config_file has incompatible type"
…mplify argument passing

why: Argparse always adds attributes to namespace with defaults, making hasattr checks redundant
what:
- Remove all hasattr checks for argparse attributes
- Simplify sync call by directly accessing args attributes
- Remove intermediate dictionary building for add and add_from_fs
- Pass arguments directly to functions instead of using **kwargs
- Cleaner and more readable code

refs: All argparse arguments have defaults (None, False, or empty list) so attributes always exist
why: Ensure consistent code formatting
what:
- Remove unused yaml import
- Fix trailing whitespace
- Add newline at end of file
why: Test suite was failing when run together due to fixture conflicts
what:
- Add vcspull.cli.fmt to list of loggers cleared in fixtures
- Rename clear_logging_handlers to reset_logging in test_fmt.py to avoid conflicts
- Update test_fmt.py tests to use reset_logging fixture

refs: Fixture isolation issue - clear_logging_handlers with autouse=True affects all tests
why: vcspull.cli.fmt was missing from the list of CLI loggers that get
SimpleLogFormatter, causing it to fall back to DebugLogFormatter which
outputs to stderr and bypasses pytest's log capture.

what:
- Add 'vcspull.cli.fmt' to cli_loggers list in setup_logger function
- Ensures fmt module gets same clean output formatting as other CLI modules

refs: Fixes test failures in test_fmt.py when run after test_log.py
…erference

why: Tests were failing when run after test_log.py because setup_logger()
created StreamHandlers that bypassed pytest's log capture mechanism.

what:
- Update reset_logging fixture to clear handlers before AND after tests
- Re-enable log propagation so pytest can capture logs properly
- Add libvcs to list of loggers to clear
- Make fixture autouse and remove unnecessary parameters from test methods
- Match the pattern used in test_add.py and test_add_from_fs.py

refs: Fixes all 7 failing tests in TestFormatConfigFile class
why: Users need a way to format all their vcspull configuration files at once,
similar to how sync discovers and processes all configs.

what:
- Add --all flag to fmt command argument parser
- Refactor format_config_file() into format_single_config() for individual files
- Update format_config_file() to handle both single file and --all modes
- Discover configs using find_config_files(include_home=True) when --all is used
- Include local .vcspull.yaml/json files in current directory
- Add clear progress reporting showing all files found and their status
- Add summary at the end showing success count
- Update existing tests to pass new format_all parameter
- Add comprehensive test for --all functionality with mocked config discovery
- Fix type annotations for test helper functions

refs: Enhancement to make vcspull fmt more powerful for managing multiple configs
Applied automatic fixes from ruff including:
- Remove unused variables and replace with underscore prefix
- Fix import ordering
- Remove trailing whitespace
- Add trailing commas
- Fix type annotation formatting
- Simplify conditional logic
why: Address B007, SIM102, PERF102, and E501 linting violations to maintain code quality standards.
what:
- Replace unused loop variable with underscore prefix
- Combine nested if statements using logical and operators
- Use values() instead of items() when keys are not needed
- Split long conditional statement across multiple lines for readability

refs: ruff check compliance
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