refactor: comprehensive cleanup and interface improvements#56
Merged
Conversation
This commit performs a comprehensive cleanup of deprecated/unused code and improves the Tool interface design: **Removed Deprecated/Unused Code:** - Removed deprecated InstallTool(), InstallTools(), InstallToolsParallel(), InstallToolsWithOptions() methods - Removed unused InstallOptions struct (replaced with EnsureTools/EnsureTool) - Removed redundant Name() methods from all tools (consolidated on GetToolName()) - Removed deprecated Pre/Post hook fields from CommandConfig (part of removed built-in command system) - Removed unused ClearAllCaches(), ClearPathCache() methods and CacheManager interface - Updated all callers to use EnsureTools/EnsureTool instead of deprecated methods **Enhanced GetChecksum Interface:** - Updated GetChecksum signature from (version, filename) to (version, cfg, filename) - Provides tools access to full ToolConfig including distribution info and options - Enables more efficient and context-aware checksum fetching - Updated all tool implementations: JavaTool, GoTool, NodeTool, MavenTool, MvndTool - Updated call sites in download.go and tests **Removed Unused Internal Methods:** - Removed getChecksumURL() from JavaTool, GoTool, NodeTool (not used) - Removed getVersionsURL() from all tools (not used anywhere) - Only kept getChecksumURL() in MavenTool and MvndTool where actually used **Interface Simplification:** - Eliminated entire ToolMetadataProvider interface - Moved GetDisplayName() into main Tool interface - Removed GetEmoji() method entirely (using default emoji in display code) - Made GetChecksumURL() and GetVersionsURL() private implementation details **Java Checksum Architecture Clarification:** - Java checksums are handled during installation via getDownloadURLWithChecksum() - GetChecksum() method returns informative error since checksums are pre-fetched - Maintains existing efficient integration with Disco API during URL resolution **Benefits:** - Cleaner Tool interface with only necessary public methods - Removed 243+ lines of deprecated/unused code - More consistent and extensible GetChecksum signature - Better separation of concerns between installation-time and on-demand checksum fetching - Improved maintainability by eliminating dead code - Follows SOLID principles with focused interfaces All tests pass and functionality is preserved while significantly reducing code complexity.
76baf9a to
42b2e28
Compare
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
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.
Overview
This PR performs a comprehensive cleanup of deprecated/unused code and improves the Tool interface design, resulting in a significantly cleaner and more maintainable codebase.
Changes Made
🧹 Removed Deprecated/Unused Code
Deprecated Installation Methods:
InstallTool(),InstallTools(),InstallToolsParallel(),InstallToolsWithOptions()InstallOptionsstructEnsureTools()/EnsureTool()insteadRedundant Tool Methods:
Name()methods from all toolsGetToolName()for consistencyDeprecated Config Fields:
Pre/Posthook fields fromCommandConfigUnused Cache Methods:
ClearAllCaches(),ClearPathCache()methodsCacheManagerinterface🔧 Enhanced GetChecksum Interface
Before:
After:
Benefits:
ToolConfigincluding distribution info and optionscfg.Distributionto target specific distributions (temurin, zulu, etc.)🧹 Removed Unused Internal Methods
Removed from all tools:
getVersionsURL()- Not used anywhere in the codebasegetChecksumURL()from JavaTool, GoTool, NodeTool - Not actually calledKept where actually used:
getChecksumURL()in MavenTool and MvndTool - Still called by theirGetChecksum()methods🔍 Interface Simplification
Eliminated Entire Interface:
ToolMetadataProviderinterface entirelyGetDisplayName()into mainToolinterfaceGetEmoji()method (using default emoji "📦" in display code)Made Implementation Details Private:
GetChecksumURL()andGetVersionsURL()private (lowercase) where not used📋 Updated All Tool Implementations
🔍 Java Checksum Architecture Clarification
Java has a different checksum architecture than other tools:
Other Tools (Go, Node, Maven, Mvnd):
GetChecksum()fetches checksums dynamically when neededJava Tool:
getDownloadURLWithChecksum()GetChecksum()returns informative error since checksums are pre-fetchedFiles Changed
cmd/root.go- Updated to use EnsureTools instead of deprecated methodscmd/setup.go- Updated to use EnsureTools instead of deprecated methodscmd/tools.go- Updated display logic for simplified interfacepkg/config/config.go- Removed deprecated Pre/Post hook fieldspkg/tools/base_tool.go- Removed deprecated methods and cache interfacespkg/tools/manager.go- Updated Tool interface, removed deprecated methodspkg/tools/java.go- Updated signature, removed unused methods, clarified architecturepkg/tools/go.go- Updated signature, removed unused methodspkg/tools/node.go- Updated signature, removed unused methodspkg/tools/maven.go- Updated signature, removed unused getVersionsURLpkg/tools/mvnd.go- Updated signature, removed unused getVersionsURLpkg/tools/download.go- Updated GetChecksum call, removed fallback logicpkg/tools/checksum_test.go- Updated tests for new signatureTesting
Impact
Code Quality Improvements
Name()methods in favor of consistentGetToolName()Migration Path
All deprecated methods have been replaced with their modern equivalents:
InstallTool*()→EnsureTool()/EnsureTools()tool.Name()→tool.GetToolName()ToolMetadataProvider→ Directtool.GetDisplayName()callsThis cleanup significantly improves code maintainability while preserving all existing functionality.