Conversation
Adds searchTerm to DecisionRequest, skipFilters to Placement, group to User, and new SkipFilters model.
Bump typhoeus ~> 1.4, rspec ~> 3.12, rake ~> 13.2, and rubocop ~> 1.50. This also fixes the jaro_winkler build failure on Ruby 3.2+.
Bump ruby-version from 2.6 to 3.2 and actions/checkout from v2 to v4 across both CI and release workflows. Use bundler-cache in the build job.
There was a problem hiding this comment.
Generally speaking I think this good to go.
A few things I want to run through @akiradev:
- We have a few version mismatches. Might need attention/confirmation.
- We don't have tests covering any basics. May not be needed at all since we are coming from code generated from specs and that would create additional work.
Around version, this is what I think what needs decision making:
https://github.com/adzerk/adzerk-decision-sdk-ruby/pull/27/changes#r2788734493
PS: Running bundle exec rubocop shows a few warnings that might be nice to fix.
PS2: I think we should bump the version in lib/adzerk_decision_sdk/version.rb, event if it is from 1.0.0-beta.13 to 1.0.0-beta.14
| s.required_ruby_version = ">= 2.4" | ||
|
|
||
| s.add_runtime_dependency 'typhoeus', '~> 1.0', '>= 1.0.1' | ||
| s.add_runtime_dependency 'typhoeus', '~> 1.4' |
There was a problem hiding this comment.
[far question] It seems that for broader support it might be better to stick to what we had before.
There was a problem hiding this comment.
After checking the ecosystem we have here and with and eye on supporting as many clients as possible, I think that we want:
- ruby
>= 2.7(unless we want a maintained version, in which case, I would go with3.2.2) - typhoeus
'~> 1.0', >= 1.0.1'(unless we want a maintained version, in which case, I would go with'~> 1.4'✅) - rspec
'~> 3.12'✅
There was a problem hiding this comment.
I updated the minimum Ruby version based on https://www.ruby-lang.org/en/downloads/branches/. Even Ruby 3.2 is nearing EOL. That said, please let me know the current distribution of Ruby versions among SDK users, and I'll happily make adjustments.
| uses: ruby/setup-ruby@v1 | ||
| with: | ||
| ruby-version: '2.6' | ||
| ruby-version: '3.2' |
There was a problem hiding this comment.
[far fn]Based on this comment around ruby version, we should:
- at a minimum, match that version here.
- run on tests on a few versions, if applicable. Cursor.IA suggests these:
- 2.7,
- 3.0,
- 3.1, and
- 3.2
Depending on the picked version we would only test on 3.2 or we could replicate the tests on the suggested versions.
| uses: ruby/setup-ruby@v1 | ||
| with: | ||
| ruby-version: '2.6' | ||
| ruby-version: '3.2' |
There was a problem hiding this comment.
Similarly to this comment, though is this case, we just pick the lowest version we want to support.
| ruby-version: '2.6' | ||
| ruby-version: '3.2' | ||
| - name: Update Version | ||
| run: sed -i -r "s/VERSION = '[0-9a-z.-]+'\$/VERSION = '0.0.1.${GITHUB_REF##*/}'/g" lib/adzerk_decision_sdk/version.rb |
There was a problem hiding this comment.
[far question] Is there a reason not to use github.event.release.tag_name as you did for Android?
| uses: ruby/setup-ruby@v1 | ||
| with: | ||
| ruby-version: '2.6' | ||
| ruby-version: '3.2' |
There was a problem hiding this comment.
Similarly to this comment, though is this case, we just pick the lowest version we want to support.
3 participants
searchTermfield onDecisionRequestfor targeting ads based on shopper search termsskipFiltersfield onPlacementwith newSkipFiltersmodelgroupfield onUserfor assigned cohort