Skip to content

Conversation

JuanVqz
Copy link
Member

@JuanVqz JuanVqz commented Oct 17, 2025

Summary

This PR refactors the Skunk analysis, integrating its methods directly into RubyCritic::AnalysedModulesCollection. This approach provides better encapsulation and follows Ruby's module-based architecture patterns.

Changes

Enhanced RubyCritic::AnalysedModulesCollection: Added Skunk-specific methods directly to the collection:

  • skunk_score - Calculate total Skunk score for all modules
  • average_skunk_score - Calculate average Skunk score
  • worst_modules - Find modules with highest Skunk scores
  • test_modules - Filter test modules from the collection
  • to_h - Convert collection to hash format for JSON serialization
    Updated dependent classes: Modified all classes that previously used the Analysis class:
  • StatusReporter: Now uses collection methods directly
  • Html::Overview: Simplified by using collection methods
  • Json::Simple: Uses collection's to_h method for consistent data structure

Benefits

Better encapsulation: Skunk analysis logic is now part of the RubyCritic ecosystem
Reduced complexity: Eliminates the need for a separate analysis class
Improved maintainability: Changes to analysis logic are centralized in the collection
Consistent API: All report generators now use the same collection-based interface
Follows Ruby patterns: Uses module-based architecture instead of standalone classes

Testing

• All existing tests pass
• Updated test suite to work with the new module-based approach
• Maintained comprehensive coverage of all Skunk analysis functionality

This refactoring improves code organization by integrating Skunk analysis directly into the RubyCritic framework while maintaining all existing functionality.

@JuanVqz JuanVqz marked this pull request as draft October 17, 2025 16:37
@JuanVqz JuanVqz changed the title Add Skunk::Analysis class to centralize business logic Add Skunk::Analysis class to centralize logic Oct 17, 2025
@JuanVqz JuanVqz changed the title Add Skunk::Analysis class to centralize logic Centralize Skunk Analysis Logic Oct 17, 2025
Refactor Skunk analysis integration by removing the Analysis class and adding its methods directly to RubyCritic::AnalysedModulesCollection. Update related files to utilize the new methods for Skunk score calculations and reporting.
@JuanVqz JuanVqz changed the title Centralize Skunk Analysis Logic Refactor Skunk Analysis: Move from class to module Oct 17, 2025
@JuanVqz JuanVqz requested a review from Copilot October 17, 2025 17:29
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors Skunk analysis by moving functionality from a separate Analysis class into RubyCritic::AnalysedModulesCollection as instance methods. This provides better encapsulation and follows Ruby's module-based architecture patterns.

  • Adds Skunk-specific methods directly to AnalysedModulesCollection for calculating scores and filtering modules
  • Updates dependent classes (StatusReporter, Html::Overview, Json::Simple) to use collection methods instead of separate analysis class
  • Removes the standalone SkunkData class and integrates its functionality into the collection

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/test_helper.rb Adds require for skunk gem
test/lib/skunk/rubycritic/analysed_modules_collection_test.rb New comprehensive test suite for collection methods
test/lib/skunk/generators/html/path_truncator_test.rb Removes rubocop disable comments
test/lib/skunk/config_test.rb Adds documentation comments and clarifies duplicate test
test/lib/skunk/commands/status_reporter_test.rb Adds require for analysed_modules_collection
lib/skunk/rubycritic/analysed_modules_collection.rb Monkey-patches collection with Skunk analysis methods
lib/skunk/generators/json/simple.rb Simplifies by using collection's to_hash method
lib/skunk/generators/html/templates/skunk_overview.html.erb Updates template to use collection methods directly
lib/skunk/generators/html/skunk_data.rb Removes entire file (functionality moved to collection)
lib/skunk/generators/html/overview.rb Refactored to use collection methods instead of SkunkData
lib/skunk/commands/status_reporter.rb Delegates analysis methods to collection
.rubocop_todo.yml Updates timestamp and block length metrics
.reek.yml Adds exclusions for new test methods

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

def test_add_format_ignores_duplicates
Config.add_format(:html)
Config.add_format(:html)
Config.add_format(:html) # This should be ignored as duplicate
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The inline comment is redundant since the test method name 'test_add_format_ignores_duplicates' and the assertion already make the intent clear.

Suggested change
Config.add_format(:html) # This should be ignored as duplicate
Config.add_format(:html)

Copilot uses AI. Check for mistakes.

JuanVqz and others added 3 commits October 17, 2025 11:30
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@JuanVqz JuanVqz marked this pull request as ready for review October 17, 2025 17:38
@JuanVqz JuanVqz changed the title Refactor Skunk Analysis: Move from class to module Refactor Skunk Analysis Oct 17, 2025
@JuanVqz JuanVqz changed the title Refactor Skunk Analysis Refactor Skunk Analysis: centralize Skunk attributes Oct 17, 2025
@JuanVqz
Copy link
Member Author

JuanVqz commented Oct 17, 2025

Since now we have multiple reports, JSON, Console, HTML we need to have a single point of truth to calculate skunk scores, and in the first try (commit) I was thinking on having them in a class Analysis but then quickly discovered (second commit) including them in the Collection module was the right movement, since that is always available to create any reports.

I hope that also tracks on you, and let me know if you find a better way to manage the Skunk Scores

Chears,
🦨

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