Skip to content

fix: use provider object as hash key in ProviderStateRegistry#270

Open
cristiangmarta wants to merge 1 commit intoopen-feature:mainfrom
cristiangmarta:fix/provider-state-registry-object-id
Open

fix: use provider object as hash key in ProviderStateRegistry#270
cristiangmarta wants to merge 1 commit intoopen-feature:mainfrom
cristiangmarta:fix/provider-state-registry-object-id

Conversation

@cristiangmarta
Copy link
Copy Markdown

@cristiangmarta cristiangmarta commented Apr 16, 2026

Summary

  • Replace provider.object_id as the hash key in ProviderStateRegistry with compare_by_identity, making the internal @states hash use object identity for key lookup
  • Remove the now-unnecessary object_id: stubs from test doubles in provider_state_registry_spec.rb

Motivation

The original code used provider.object_id as an integer key to track provider state by identity. This caused Ruby 4.0 warnings in tests:

warning: redefining 'object_id' may cause serious problems

Using {}.compare_by_identity is the idiomatic solution, it preserves the original semantics without calling object_id at all.

@cristiangmarta cristiangmarta requested a review from a team as a code owner April 16, 2026 23:00
Copilot AI review requested due to automatic review settings April 16, 2026 23:00
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the ProviderStateRegistry to use the provider object itself as the key in the internal states hash, replacing the previous usage of provider.object_id. Corresponding updates were made to the test suite to remove unnecessary object_id stubs from provider doubles. I have no feedback to provide.

Copy link
Copy Markdown

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 updates ProviderStateRegistry to key provider state by the provider object itself (instead of provider.object_id) to avoid Ruby 4.0 warnings caused by stubbing object_id in tests.

Changes:

  • Switch ProviderStateRegistry internal @states hash keys from provider.object_id to provider.
  • Update provider-state registry specs to stop stubbing object_id on doubles.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
spec/open_feature/sdk/provider_state_registry_spec.rb Removes object_id stubs from provider doubles used in registry specs.
lib/open_feature/sdk/provider_state_registry.rb Uses provider objects as hash keys for storing/retrieving/removing provider state.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/open_feature/sdk/provider_state_registry.rb
Copy link
Copy Markdown

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/open_feature/sdk/provider_state_registry.rb
@cristiangmarta cristiangmarta marked this pull request as draft April 16, 2026 23:21
…ider.object_id

Signed-off-by: Cristian Marta <cristian@caffe-lento.com>
Copy link
Copy Markdown

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread spec/open_feature/sdk/provider_state_registry_spec.rb
@cristiangmarta cristiangmarta marked this pull request as ready for review April 16, 2026 23:41
@cristiangmarta cristiangmarta requested a review from Copilot April 17, 2026 05:47
Copy link
Copy Markdown

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants