New commands: add and add-from-fs
#465
Draft
Conversation
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
Reviewer's GuideThis pull request introduces two new CLI subcommands, File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
tony
changed the title
add and add-from-fsadd and add-from-fs
tony
force-pushed
the
scanner-and-add
branch
2 times, most recently
from
a36fbef to
69a222e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #465 +/- ##
==========================================
- Coverage 78.93% 75.50% -3.43%
==========================================
Files 8 10 +2
Lines 413 641 +228
Branches 85 137 +52
==========================================
+ Hits 326 484 +158
- Misses 52 105 +53
- Partials 35 52 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tony
force-pushed
the
scanner-and-add
branch
3 times, most recently
from
d37cb7e to
628830e
Compare
tony
force-pushed
the
scanner-and-add
branch
3 times, most recently
from
e85482d to
e26036d
Compare
tony
force-pushed
the
scanner-and-add
branch
5 times, most recently
from
ef61940 to
740c98c
Compare
Make config parameter in sync function explicitly optional in type signature to match actual behavior. This improves type checking without changing functionality.
- Fix sync() function call to pass pathlib.Path for config parameter - Add save_config_yaml to helpers and fix imports - Improve test teardown to use shutil.rmtree - Add missing pathlib import in CLI __init__.py - Set logging level in tests for proper log capture
The sync() function requires config as a parameter even when None. Update the kwargs filtering to always include config in the allowed parameters list.
- Fix line length issues (E501) in CLI modules - Replace open() with Path.open() (PTH123) - Remove redundant exception objects in logging.exception() calls (TRY401) - Break long strings across multiple lines for better readability - All ruff and mypy checks now pass for the new CLI commands
- Add save_config_yaml to vcspull.config module using ConfigReader._dump - Remove duplicate implementations from cli/add.py and cli/add_from_fs.py - Update imports to use the centralized function - Remove duplicate from test helpers - Follows DRY principle and provides consistent YAML formatting
- Replace complex repo_name_or_url parsing with explicit name and url args - Remove 150+ lines of confusing comments and logic - Add clear --path and --dir options for specifying location - Make command usage intuitive: vcspull add <name> <url> - Reduce add_repo function from 270 to 163 lines - Improve help text and parameter documentation
… patterns why: Ensure consistency with project patterns and improve code maintainability. what: - Updated module and function docstrings to match NumPy style - Cleaned up imports and removed unnecessary comments - Fixed error handling to match sync.py patterns - Fixed type annotations for mypy compliance - Fixed non-recursive scanning bug in add_from_fs.py - Removed unused helper functions in tests
…notation why: Fix mypy error for variable redefinition on line 201. what: - Remove redundant type annotation for determined_base_key in non-recursive branch - Variable was already declared with type annotation in recursive branch
…ositories why: Users need visibility into which repositories already exist in configuration and where they are located. what: - Show count and detailed list of existing repositories when found - Display repository name, URL, config path, and file location for each existing repo - Simplify logic by consolidating duplicate repository checking code - Improve user experience with more informative output
… existing repo reporting why: Ensure the enhanced output functionality works correctly across different scenarios. what: - Update existing test to verify new detailed output format - Add test for multiple existing repositories with detailed listing - Add test for mixed scenario with both existing and new repositories - Verify count, individual repo details, and final status messages - Test output includes repo name, URL, config path, and file location
…ommands why: CLI add/add-from-fs commands should have clean output like print() while preserving debug output for VCS operations. what: - Add SimpleLogFormatter that outputs only the message without extra formatting - Configure specific loggers for vcspull.cli.add and vcspull.cli.add_from_fs to use simple formatter - Keep DebugLogFormatter for vcspull core operations and sync commands - Prevent logger propagation to avoid duplicate output - Maintain detailed VCS operation logging for sync functionality
…ilities why: Ensure proper testing coverage for all logging formatters and setup functionality what: - Add test_log.py with comprehensive test coverage for all log components - Test LogFormatter, DebugLogFormatter, SimpleLogFormatter, RepoLogFormatter classes - Test RepoFilter functionality and setup_logger behavior - Test integration scenarios with actual loggers - Follow vcspull testing patterns with NamedTuple parametrized tests - Include proper pytest fixtures for logger cleanup
…rting why: Follow best practices for exception logging and provide better debugging info what: - Replace log.error with log.exception in exception handlers - Maintain existing debug traceback functionality - Apply to both add.py and add_from_fs.py modules
why: Ensure code passes linting and follows project formatting standards what: - Fix line length violations by splitting long strings - Improve code readability with proper line breaks - Use dict() constructor for cleaner code - Remove unused loop variables with underscore prefix
… clean output why: Ensure CLI sync command provides clean user-facing output like print() what: - Extend SimpleLogFormatter to vcspull.cli.sync logger - Consolidate CLI logger setup with DRY loop pattern - Maintain clean output for all user-facing CLI commands
…e stderr why: SimpleLogFormatter outputs to stderr via StreamHandler, but test only captured stdout what: - Update output capture to include both stdout and stderr - Use iterable unpacking for cleaner code style - Ensure all CLI log messages are properly captured in tests
…ync logger why: Ensure CLI sync logger is properly configured with SimpleLogFormatter what: - Add test_setup_logger_creates_cli_sync_logger test case - Update logger cleanup fixture to include sync logger - Update all setup_logger tests to clear sync logger handlers - Verify sync logger uses SimpleLogFormatter for clean output
why: Ensure code meets ruff line length limit of 88 characters what: - Break long f-string expressions across multiple lines - Split colorama formatting codes at natural boundaries - Maintain readability while meeting style requirements refs: E501 line length violations fixed
why: The create_parser function was over-engineered with complex overloads and dictionary returns when a simple tuple would suffice. what: - Remove get_all_subparsers parameter and its complex overload - Return simple tuple (parser, subparsers_tuple) instead of dictionary - Use direct tuple unpacking in cli() function - Remove unnecessary dictionary lookups for subparsers - Maintain backward compatibility with existing return pattern This follows the existing vcspull pattern of keeping things simple and direct rather than adding unnecessary abstraction layers.
why: When scanning directories with many existing repos (37+), the output was overwhelming and buried the important information about new repos. what: - Show summary only when >5 existing repos instead of full list - Add clear "Found X new repositories to add:" section before confirmation - Display new repos prominently with + prefix before asking to add - Keep detailed output for ≤5 existing repos - Add test for many existing repos scenario This dramatically improves signal-to-noise ratio when adding repos to large existing configurations. refs: User feedback about 37-39 repo output drowning out 2 new additions
why: Simple string format doesn't allow for future extensions like
multiple remotes or other repo-specific settings.
what:
- Change add and add-from-fs to write verbose format with repo: key
- Use {"repo": "url"} instead of plain string format
- Add comprehensive tests for add command
- Update all test assertions to expect verbose format
This matches vcspull's existing test patterns and README examples,
and allows users to easily extend configs with remotes later.
…ng duplicates
why: The duplicate repo check was showing the raw dict structure
{'repo': 'url'} instead of just the URL when checking verbose format
configs, causing test failures.
what:
- Extract URL from both string and dict config formats
- Check for both 'repo' and 'url' keys in dict configs
- Display clean URL in warning messages
- Fix line length issues flagged by ruff
- Fix mypy type error with proper type annotation
This ensures consistent user experience regardless of whether existing
configs use simple string format or verbose dict format.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes:
add-from-fs#25Summary by Sourcery
Add new CLI commands
addandadd-from-fsto vcspull for managing repository configurationNew Features:
addcommand to add a single repository to vcspull configurationadd-from-fscommand to scan and add multiple git repositories from a directoryEnhancements: