Refactor to use Absmartly as top module#5
Draft
cwmpinho wants to merge 2 commits into
Draft
Conversation
Collaborator
Author
|
@copilot Implement the refactor detailed in REFACTOR_PLAN.md |
* Initial plan * Refactor: move all lib files into lib/absmartly/ namespace - Move all lib/*.rb files into lib/absmartly/ directory - Wrap all class/module definitions in module Absmartly - Keep String and Array monkey-patches unwrapped - Update lib/absmartly.rb to use new require paths - Bump VERSION from 1.3.0 to 2.0.0 - Update all spec files to use Absmartly:: prefix and absmartly/ require paths - Remove old lib/ files (moved to lib/absmartly/) - All 261 tests pass Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Refactor: namespace all classes under Absmartly module, bump version to 2.0.0 Co-authored-by: cwmpinho <154367772+cwmpinho@users.noreply.github.com> * Remove vendor/bundle from git tracking Co-authored-by: cwmpinho <154367772+cwmpinho@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: cwmpinho <154367772+cwmpinho@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors the gem to avoid polluting the global Ruby namespace by moving the implementation under the Absmartly top-level module (and corresponding lib/absmartly/** layout), updating specs accordingly, and bumping the gem major version.
Changes:
- Move/namespace the implementation from top-level constants into
module Absmartlywith alib/absmartly/**directory structure. - Update specs to require namespaced paths and reference
Absmartly::*constants/classes. - Bump version to
2.0.0and add a written refactor plan.
Reviewed changes
Copilot reviewed 150 out of 153 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/variant_assigner_spec.rb | Update requires and references to Absmartly::VariantAssigner. |
| spec/json_expr/operators/var_operator_spec.rb | Update requires and references to Absmartly::VarOperator. |
| spec/json_expr/operators/value_operator_spec.rb | Update requires and references; use Absmartly::EMPTY_MAP/LIST. |
| spec/json_expr/operators/shared_operator.rb | Update evaluator require and use Absmartly::Evaluator. |
| spec/json_expr/operators/or_combinator_spec.rb | Update requires and references to Absmartly::OrCombinator. |
| spec/json_expr/operators/not_operator_spec.rb | Update requires and references to Absmartly::NotOperator. |
| spec/json_expr/operators/nil_operator_spec.rb | Update requires and references to Absmartly::NilOperator. |
| spec/json_expr/operators/match_operator_spec.rb | Update requires and references to Absmartly::MatchOperator. |
| spec/json_expr/operators/less_than_or_equal_operator_spec.rb | Update requires and references to Absmartly::LessThanOrEqualOperator. |
| spec/json_expr/operators/less_than_operator_spec.rb | Update requires and references to Absmartly::LessThanOperator. |
| spec/json_expr/operators/in_operator_spec.rb | Update requires and references to Absmartly::InOperator. |
| spec/json_expr/operators/greater_than_or_equal_operator_spec.rb | Update requires and references to Absmartly::GreaterThanOrEqualOperator. |
| spec/json_expr/operators/greater_than_operator_spec.rb | Update requires and references to Absmartly::GreaterThanOperator. |
| spec/json_expr/operators/equals_operator_spec.rb | Update requires and references to Absmartly::EqualsOperator. |
| spec/json_expr/operators/and_combinator_spec.rb | Update requires and references to Absmartly::AndCombinator. |
| spec/json_expr/json_expr_spec.rb | Update requires and references to Absmartly::JsonExpr. |
| spec/json_expr/expr_evaluator_spec.rb | Update requires and references to Absmartly::ExprEvaluator and related constants/types. |
| spec/hashing_spec.rb | Update requires and references to Absmartly::Hashing. |
| spec/default_variable_parser_spec.rb | Update requires and references to Absmartly::DefaultVariableParser and Absmartly::Context. |
| spec/default_http_client_spec.rb | Update requires and references to Absmartly::DefaultHttpClient and config. |
| spec/default_http_client_config_spec.rb | Update requires and references to Absmartly::DefaultHttpClientConfig. |
| spec/default_context_data_deserializer_spec.rb | Update requires and references to namespaced JSON model classes. |
| spec/default_audience_deserializer_spec.rb | Update requires and references to Absmartly::DefaultAudienceDeserializer. |
| spec/context_spec.rb | Update requires and references throughout to Absmartly::*. |
| spec/context_config_spec.rb | Update requires and references to Absmartly::ContextConfig. |
| spec/client_spec.rb | Update requires and references to Absmartly::Client and related types. |
| spec/client_config_spec.rb | Update requires and references to Absmartly::ClientConfig and related interfaces. |
| spec/audience_matcher_spec.rb | Update requires and references to Absmartly::AudienceMatcher. |
| spec/absmartly_spec.rb | Update requires and references to Absmartly::* types used by the module-level API. |
| spec/a_b_smartly_spec.rb | Update requires and references to Absmartly::ABSmartly and config. |
| spec/a_b_smartly_config_spec.rb | Update requires and references to Absmartly::ABSmartlyConfig and dependencies. |
| lib/variant_assigner.rb | Deleted (moved/rewritten under lib/absmartly/variant_assigner.rb). |
| lib/variable_parser.rb | Deleted (moved under lib/absmartly/variable_parser.rb). |
| lib/scheduled_thread_pool_executor.rb | Deleted (moved under lib/absmartly/scheduled_thread_pool_executor.rb). |
| lib/scheduled_executor_service.rb | Deleted (moved under lib/absmartly/scheduled_executor_service.rb). |
| lib/json_expr/operators/var_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/var_operator.rb). |
| lib/json_expr/operators/value_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/value_operator.rb). |
| lib/json_expr/operators/unary_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/unary_operator.rb). |
| lib/json_expr/operators/or_combinator.rb | Deleted (moved under lib/absmartly/json_expr/operators/or_combinator.rb). |
| lib/json_expr/operators/not_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/not_operator.rb). |
| lib/json_expr/operators/nil_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/nil_operator.rb). |
| lib/json_expr/operators/match_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/match_operator.rb). |
| lib/json_expr/operators/less_than_or_equal_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/less_than_or_equal_operator.rb). |
| lib/json_expr/operators/less_than_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/less_than_operator.rb). |
| lib/json_expr/operators/in_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/in_operator.rb). |
| lib/json_expr/operators/greater_than_or_equal_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/greater_than_or_equal_operator.rb). |
| lib/json_expr/operators/greater_than_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/greater_than_operator.rb). |
| lib/json_expr/operators/equals_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/equals_operator.rb). |
| lib/json_expr/operators/boolean_combinator.rb | Deleted (moved under lib/absmartly/json_expr/operators/boolean_combinator.rb). |
| lib/json_expr/operators/binary_operator.rb | Deleted (moved under lib/absmartly/json_expr/operators/binary_operator.rb). |
| lib/json_expr/operators/and_combinator.rb | Deleted (moved under lib/absmartly/json_expr/operators/and_combinator.rb). |
| lib/json_expr/operator.rb | Deleted (moved under lib/absmartly/json_expr/operator.rb). |
| lib/json_expr/json_expr.rb | Deleted (moved under lib/absmartly/json_expr/json_expr.rb). |
| lib/json_expr/expr_evaluator.rb | Deleted (moved under lib/absmartly/json_expr/expr_evaluator.rb). |
| lib/json_expr/evaluator.rb | Deleted (moved under lib/absmartly/json_expr/evaluator.rb). |
| lib/json/unit.rb | Deleted (moved under lib/absmartly/json/unit.rb). |
| lib/json/publish_event.rb | Deleted (moved under lib/absmartly/json/publish_event.rb). |
| lib/json/goal_achievement.rb | Deleted (moved under lib/absmartly/json/goal_achievement.rb). |
| lib/json/exposure.rb | Deleted (moved under lib/absmartly/json/exposure.rb). |
| lib/json/experiment_variant.rb | Deleted (moved under lib/absmartly/json/experiment_variant.rb). |
| lib/json/experiment_application.rb | Deleted (moved under lib/absmartly/json/experiment_application.rb). |
| lib/json/experiment.rb | Deleted (moved under lib/absmartly/json/experiment.rb). |
| lib/json/custom_field_value.rb | Deleted (moved under lib/absmartly/json/custom_field_value.rb). |
| lib/json/context_data.rb | Deleted (moved under lib/absmartly/json/context_data.rb). |
| lib/json/attribute.rb | Deleted (moved under lib/absmartly/json/attribute.rb). |
| lib/http_client.rb | Deleted (moved under lib/absmartly/http_client.rb). |
| lib/hashing.rb | Deleted (moved under lib/absmartly/hashing.rb). |
| lib/default_variable_parser.rb | Deleted (moved under lib/absmartly/default_variable_parser.rb). |
| lib/default_http_client_config.rb | Deleted (moved under lib/absmartly/default_http_client_config.rb). |
| lib/default_http_client.rb | Deleted (moved under lib/absmartly/default_http_client.rb). |
| lib/default_context_event_serializer.rb | Deleted (moved under lib/absmartly/default_context_event_serializer.rb). |
| lib/default_context_event_handler.rb | Deleted (moved under lib/absmartly/default_context_event_handler.rb). |
| lib/default_context_data_provider.rb | Deleted (moved under lib/absmartly/default_context_data_provider.rb). |
| lib/default_context_data_deserializer.rb | Deleted (moved under lib/absmartly/default_context_data_deserializer.rb). |
| lib/default_audience_deserializer.rb | Deleted (moved under lib/absmartly/default_audience_deserializer.rb). |
| lib/context_event_serializer.rb | Deleted (moved under lib/absmartly/context_event_serializer.rb). |
| lib/context_event_logger_callback.rb | Deleted (moved under lib/absmartly/context_event_logger_callback.rb). |
| lib/context_event_logger.rb | Deleted (moved under lib/absmartly/context_event_logger.rb). |
| lib/context_event_handler.rb | Deleted (moved under lib/absmartly/context_event_handler.rb). |
| lib/context_data_provider.rb | Deleted (moved under lib/absmartly/context_data_provider.rb). |
| lib/context_data_deserializer.rb | Deleted (moved under lib/absmartly/context_data_deserializer.rb). |
| lib/context_config.rb | Deleted (moved under lib/absmartly/context_config.rb). |
| lib/context.rb | Deleted (moved under lib/absmartly/context.rb). |
| lib/client_config.rb | Deleted (moved under lib/absmartly/client_config.rb). |
| lib/client.rb | Deleted (moved under lib/absmartly/client.rb). |
| lib/audience_matcher.rb | Deleted (moved under lib/absmartly/audience_matcher.rb). |
| lib/audience_deserializer.rb | Deleted (moved under lib/absmartly/audience_deserializer.rb). |
| lib/absmartly/version.rb | Bump gem version to 2.0.0. |
| lib/absmartly/variant_assigner.rb | Add Absmartly::VariantAssigner implementation under namespace. |
| lib/absmartly/variable_parser.rb | Add Absmartly::VariableParser interface under namespace. |
| lib/absmartly/string.rb | Move core String monkey-patch into lib/absmartly/ path (still global). |
| lib/absmartly/scheduled_thread_pool_executor.rb | Add namespaced scheduled executor implementation. |
| lib/absmartly/scheduled_executor_service.rb | Add namespaced scheduler interface. |
| lib/absmartly/json_expr/operators/var_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/value_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/unary_operator.rb | Add namespaced JSON expr operator mixin. |
| lib/absmartly/json_expr/operators/or_combinator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/not_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/nil_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/match_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/less_than_or_equal_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/less_than_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/in_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/greater_than_or_equal_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/greater_than_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/equals_operator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operators/boolean_combinator.rb | Add namespaced JSON expr combinator mixin. |
| lib/absmartly/json_expr/operators/binary_operator.rb | Add namespaced JSON expr operator mixin. |
| lib/absmartly/json_expr/operators/and_combinator.rb | Add namespaced JSON expr operator. |
| lib/absmartly/json_expr/operator.rb | Add namespaced operator interface. |
| lib/absmartly/json_expr/json_expr.rb | Add namespaced JSON expression engine entrypoint. |
| lib/absmartly/json_expr/expr_evaluator.rb | Add namespaced evaluator and EMPTY_MAP/LIST constants. |
| lib/absmartly/json_expr/evaluator.rb | Add namespaced evaluator interface. |
| lib/absmartly/json/unit.rb | Add namespaced JSON model type. |
| lib/absmartly/json/publish_event.rb | Add namespaced JSON model type. |
| lib/absmartly/json/goal_achievement.rb | Add namespaced JSON model type. |
| lib/absmartly/json/exposure.rb | Add namespaced JSON model type. |
| lib/absmartly/json/experiment_variant.rb | Add namespaced JSON model type. |
| lib/absmartly/json/experiment_application.rb | Add namespaced JSON model type. |
| lib/absmartly/json/experiment.rb | Add namespaced JSON model type. |
| lib/absmartly/json/custom_field_value.rb | Add namespaced JSON model type. |
| lib/absmartly/json/context_data.rb | Add namespaced JSON model type. |
| lib/absmartly/json/attribute.rb | Add namespaced JSON model type. |
| lib/absmartly/http_client.rb | Add namespaced HTTP client interface. |
| lib/absmartly/hashing.rb | Add namespaced hashing helper. |
| lib/absmartly/default_variable_parser.rb | Add namespaced default variable parser implementation. |
| lib/absmartly/default_http_client_config.rb | Add namespaced HTTP client config. |
| lib/absmartly/default_http_client.rb | Add namespaced Faraday-based HTTP client. |
| lib/absmartly/default_context_event_serializer.rb | Add namespaced event serializer. |
| lib/absmartly/default_context_event_handler.rb | Add namespaced event handler. |
| lib/absmartly/default_context_data_provider.rb | Add namespaced context data provider. |
| lib/absmartly/default_context_data_deserializer.rb | Add namespaced context data deserializer. |
| lib/absmartly/default_audience_deserializer.rb | Add namespaced audience deserializer. |
| lib/absmartly/context_event_serializer.rb | Add namespaced serializer interface. |
| lib/absmartly/context_event_logger_callback.rb | Add namespaced logger callback implementation. |
| lib/absmartly/context_event_logger.rb | Add namespaced event logger interface/constants. |
| lib/absmartly/context_event_handler.rb | Add namespaced event handler interface. |
| lib/absmartly/context_data_provider.rb | Add namespaced context data provider interface. |
| lib/absmartly/context_data_deserializer.rb | Add namespaced context data deserializer interface. |
| lib/absmartly/context_config.rb | Add namespaced context configuration. |
| lib/absmartly/context.rb | Add namespaced core Context implementation. |
| lib/absmartly/client_config.rb | Add namespaced client configuration. |
| lib/absmartly/client.rb | Add namespaced API client. |
| lib/absmartly/audience_matcher.rb | Add namespaced audience matcher. |
| lib/absmartly/audience_deserializer.rb | Add namespaced audience deserializer interface. |
| lib/absmartly/a_b_smartly_config.rb | Add namespaced SDK config. |
| lib/absmartly/a_b_smartly.rb | Add namespaced SDK entry class (Absmartly::ABSmartly). |
| lib/absmartly.rb | Update main entrypoint requires to new namespaced file locations. |
| lib/a_b_smartly_config.rb | Deleted (moved under lib/absmartly/a_b_smartly_config.rb). |
| lib/a_b_smartly.rb | Deleted (moved under lib/absmartly/a_b_smartly.rb). |
| REFACTOR_PLAN.md | Document the refactor rationale/plan and known out-of-scope issues. |
| Gemfile.lock | Update locked gem version from 1.3.0 to 2.0.0. |
| .gitignore | Add vendor/bundle and normalize .gemspec ignore entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+30
| @achievements = [] | ||
| @assignment_cache = {} | ||
| @assignments = {} | ||
| @clock = clock.is_a?(String) ? Time.iso8601(clock) : clock | ||
| @publish_delay = config.publish_delay |
There was a problem hiding this comment.
Context#initialize calls Time.iso8601(clock) when clock is a String, but this file does not require "time". If a consumer requires absmartly/context directly (without going through absmartly, which currently requires time elsewhere), this can raise NoMethodError for Time.iso8601. Consider requiring time in this file (or avoiding the dependency).
Comment on lines
+25
to
+27
| def compare(_, _) | ||
| raise NotImplementedError.new("You must implement extract_var method.") | ||
| end |
There was a problem hiding this comment.
Evaluator#compare raises NotImplementedError with the message "You must implement extract_var method.", which doesn’t match the method being called. This is confusing for implementers/debugging; update the message to reference compare (or a more general message).
Comment on lines
+3
to
+13
| module Absmartly | ||
| class ContextEventLoggerCallback < ContextEventLogger | ||
| attr_accessor :callable | ||
|
|
||
| def initialize(callable) | ||
| @callable = callable | ||
| end | ||
|
|
||
| def handle_event(event, data) | ||
| @callable.call(event, data) if @callable.present? | ||
| end |
There was a problem hiding this comment.
ContextEventLoggerCallback#handle_event calls @callable.present?, but present? is not part of Ruby core and there’s no ActiveSupport dependency loaded here. This will raise NoMethodError in non-Rails environments. Consider checking @callable for nil and/or respond_to?(:call), and add an explicit require_relative "context_event_logger" so the superclass constant is always defined when this file is required directly.
Comment on lines
+55
to
+60
| private | ||
| def add_tracking(url, params) | ||
| parsed = URI.parse(url) | ||
| query = parsed.query ? CGI.parse(parsed.query) : {} | ||
| query = query.merge(params) if params && params.is_a?(Hash) | ||
| parsed.query = URI.encode_www_form(query) |
There was a problem hiding this comment.
DefaultHttpClient#add_tracking uses CGI.parse, but this file never requires the stdlib cgi. In a minimal environment this can raise NameError: uninitialized constant CGI. Add require "cgi" (or avoid CGI.parse) to make this file self-contained.
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.
No description provided.