refactor: address all code review issues#9
Open
redlotusaustin wants to merge 5 commits intoblacksiders:mainfrom
Open
refactor: address all code review issues#9redlotusaustin wants to merge 5 commits intoblacksiders:mainfrom
redlotusaustin wants to merge 5 commits intoblacksiders:mainfrom
Conversation
Add new 'debug' domain category to DOMAIN_HEURISTICS with keywords covering: - Debugging tools and techniques (debug, breakpoint, trace) - Logging frameworks (logger, logging) - Performance profiling (profiler, profiling, devtools) - Monitoring and observability (sentry, datadog, newrelic) - Error tracking and diagnostics (error-tracking, diagnostic) This enables automatic skill classification for debugging-related tools and extensions.
- Add --skill-dir to override source skills directory - Add --vault-dir to override destination vault directory - Fix symlink handling to prevent source deletion - Fix shutil.move to use explicit destination path
…g/programming/security - Add 'documentation' category for code-documenter, docstrings, swagger, etc - Add 'wordpress' category for wordpress, generateblocks, gutenberg, woocommerce - Add bug-hunter/hunter/bug keywords to debug category - Add vibesec/vibe-security keywords to security category - Add find-skills to tooling category - Add mago to programming category
High Priority: - Add path validation for --skill-dir and --vault-dir arguments - Add comprehensive error handling for directory operations - Fix symlink handling to prevent source deletion Medium Priority: - Add type hints to all functions - Define magic numbers as constants (PROGRESS_LOG_THRESHOLD, BATCH_PROGRESS_INTERVAL) - Fix IndexOutOfBounds risk in unknown args check - Add unit tests (38 tests, 100% pass rate) Low Priority: - Remove artificial time.sleep() delays - Use Path.as_posix() for cross-platform paths - Add docstrings to all functions - Add __version__ and shebang - Optimize category matching with precomputed lookup dict - Add --dry-run mode for previewing changes Documentation: - Update README with new CLI options table - Add Development & Testing section - Add pyproject.toml for project configuration
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.
Summary
This PR addresses all issues identified in the comprehensive code review:
High Priority
--skill-dirand--vault-dirargumentsMedium Priority
Low Priority
time.sleep()delaysPath.as_posix()for cross-platform paths__version__and shebang--dry-runmode for previewing changesNew Features
--dry-runflag to preview changes without executing--versionflag to show version numberTesting
Breaking Changes
None - all changes are backward compatible.
Files Changed
setup.py- Complete refactor with all fixesREADME.md- Updated documentation with new CLI optionspyproject.toml- Added for project configuration.gitignore- Added for Python/Venv exclusionstests/test_setup.py- 38 comprehensive unit tests