Skip to content

Conversation

dhruvCW
Copy link
Contributor

@dhruvCW dhruvCW commented Aug 25, 2024

Description

  • Refactored the classes to allow for all the configurations to be specified via the ABSmartlyConfig object. The instance now contains a copy of the ClientConfig and intern the DefaultHttpConfig which allows for all the configuration to be setup via the ABsmartly.configure_client method.
  • Fixed rubocop violations and make the class methods in ABsmartly thread safe.
  • context_event_logger can now be directly configured with a proc or lambda which will automatically wrap it in ContextEventLoggerCallback instance.

Note

All changes should ideally be backwards compatible but have only been verified using the existing test suite.

Test

  • all unit tests pass
  • modified the error since it's no longer possible for ABSmartly.create to use another one.

@dhruvCW dhruvCW force-pushed the config_refactor branch 7 times, most recently from a0fb82c to b8759bf Compare August 25, 2024 17:56
@dhruvCW dhruvCW changed the title Config refactor Refactor ABSmartly configuration setup to allow for all the configuration to be specified through the ABSmartlyConfig object Aug 26, 2024
@dhruvCW dhruvCW marked this pull request as ready for review August 26, 2024 19:21
@hermeswaldemarin
Copy link
Contributor

Hey @dhruvCW , can you change the file lib/absmartly/version.rb with the version number 1.2 please?

@hermeswaldemarin
Copy link
Contributor

@dhruvCW , see if is possible to add this PR https://github.com/absmartly/ruby-sdk/pull/28/files together with this version, please?

@dhruvCW
Copy link
Contributor Author

dhruvCW commented Sep 3, 2024

@dhruvCW , see if is possible to add this PR https://github.com/absmartly/ruby-sdk/pull/28/files together with this version, please?

Done cherry picked the changes onto this branch and updated the version file as well 👍

@hermeswaldemarin hermeswaldemarin merged commit e7777fd into absmartly:main Sep 4, 2024
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.

3 participants