Convert timestamps to milliseconds and update Ruby compatibility #127
Conversation
| ruby-version: | ||
| - '2.0' | ||
| - '2.1' | ||
| - '2.2' |
There was a problem hiding this comment.
actually the active version is only 3.*.
<2.2 we have some dep issues.
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Mixpanel Ruby library by updating Ruby version requirements, expanding CI test coverage, and standardizing timestamp handling to use milliseconds across all event tracking and profile update methods.
- Updated minimum Ruby version from 2.0.0 to 2.3.0 and added required dependencies (
mutex_m,base64) - Expanded GitHub Actions CI to test Ruby 2.3 through 3.4 (previously only 2.0-2.6)
- Converted all timestamp fields from seconds to milliseconds for consistency with Mixpanel API expectations
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ruby.yml |
Expanded CI matrix to test Ruby 2.3-3.4, updated action versions to latest |
mixpanel-ruby.gemspec |
Updated minimum Ruby to 2.3.0, added mutex_m and base64 dependencies, updated rake to v13 |
lib/mixpanel-ruby/events.rb |
Changed timestamp from Time.now.to_f to (Time.now.to_f * 1000).to_i for milliseconds |
lib/mixpanel-ruby/tracker.rb |
Changed timestamp from Time.now.to_f to (Time.now.to_f * 1000).to_i for milliseconds |
lib/mixpanel-ruby/people.rb |
Changed timestamp from Time.now.to_f to (Time.now.to_f * 1000).to_i for milliseconds |
lib/mixpanel-ruby/groups.rb |
Changed timestamp to Time.now.to_i * 1000 for milliseconds (inconsistent with other modules) |
spec/mixpanel-ruby/events_spec.rb |
Updated test expectations to match new millisecond timestamp format |
spec/mixpanel-ruby/tracker_spec.rb |
Updated test expectations to match new millisecond timestamp format, changed URI.unescape to CGI.unescape |
spec/mixpanel-ruby/groups_spec.rb |
Updated test expectations for millisecond timestamps, changed quote style for consistency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| '$group_key' => 'TEST GROUP KEY', | ||
| '$group_id' => 'TEST GROUP ID', | ||
| '$time' => @time_now.to_i * 1000, | ||
| '$time' => (@time_now.to_f * 1000).to_i, |
There was a problem hiding this comment.
Inconsistent timestamp format in test expectations. Line 26 expects (@time_now.to_f * 1000).to_i (which preserves millisecond precision), but lines 42 and 57 expect @time_now.to_i * 1000 (which loses millisecond precision). All test expectations should use the same format: (@time_now.to_f * 1000).to_i to be consistent and match the implementation's intended behavior.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@hans-lizihan I've opened a new pull request, #128, to work on those changes. Once the pull request is ready, I'll request review from you. |
…#128) * Initial plan * Fix inconsistent timestamp format in groups_spec.rb tests Changed 9 occurrences from `@time_now.to_i * 1000` to `(@time_now.to_f * 1000).to_i` to ensure consistent millisecond precision across all test expectations. Co-authored-by: hans-lizihan <7879022+hans-lizihan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: hans-lizihan <7879022+hans-lizihan@users.noreply.github.com>
jaredmixpanel
changed the title
This pull request updates the Mixpanel Ruby library to improve compatibility with newer Ruby versions and standardizes how event timestamps are handled. The most significant changes include updating the minimum required Ruby version, expanding CI test coverage to more Ruby versions, and modifying timestamp handling to consistently use milliseconds.
Ruby compatibility and dependency updates:
2.3.0inmixpanel-ruby.gemspec, added new dependencies (mutex_m,base64), and updated development dependencies forrake..github/workflows/ruby.ymlto test against Ruby versions 2.3 through 3.4, and upgraded action versions foractions/checkoutandruby/setup-ruby.Timestamp handling standardization:
time,$time) inlib/mixpanel-ruby/events.rb,lib/mixpanel-ruby/groups.rb,lib/mixpanel-ruby/people.rb, andlib/mixpanel-ruby/tracker.rbto consistently use milliseconds since epoch. [1] [2] [3] [4] [5]Test updates for timestamp format:
spec/mixpanel-ruby/events_spec.rb,spec/mixpanel-ruby/groups_spec.rb, andspec/mixpanel-ruby/tracker_spec.rbto expect timestamps in milliseconds. [1] [2] [3] [4] [5] [6] [7]Minor test and code improvements:
URI.unescapetoCGI.unescapefor decoding base64 data in tests for better standards compliance.spec/mixpanel-ruby/groups_spec.rbfor consistent quoting style.