Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename object_id to message_id #352

Merged
merged 1 commit into from
Jan 21, 2025
Merged

Rename object_id to message_id #352

merged 1 commit into from
Jan 21, 2025

Conversation

fmang
Copy link
Contributor

@fmang fmang commented Jan 9, 2025

Ruby 3.4 prints the following message when redefining object_id:

warning: redefining 'object_id' may cause serious problems

According to fragment.rb, winrm’s object_id is the ID of the message, so let’s rename it to message_id.

message_id was not used before, and no calls to object_id referred to Ruby’s, so I’ve naively replaced all occurrences of object_id by message_id.

I do not have a Windows machine to test, but this change should be safe unless external calling code use these methods.

Ruby 3.4 prints the following message when redefining object_id:

```
warning: redefining 'object_id' may cause serious problems
```

According to fragment.rb, winrm’s object_id is the ID of the message, so
let’s rename it to message_id.

message_id was not used before, and no calls to object_id referred to
Ruby’s, so I’ve naively replaced all occurrences of object_id by
message_id.
@fwininger
Copy link
Contributor

@pcai can you take a look please 🙏 ?

@fwininger
Copy link
Contributor

@mwrock can you take a look please ? 🙏

@pcai
Copy link
Contributor

pcai commented Jan 21, 2025

Sorry for the delay, I approved the check to run and it seems safe so I'll merge it if the tests pass.

I'm unsure what our usual policy here is, but this is probably not part of an explicit public api - so I am gathering that this isn't going to be considered a breaking change, right? Even though in theory it could break someone who specifically depends on this?

@pcai
Copy link
Contributor

pcai commented Jan 21, 2025

Thank you for the contribution. Tests pass, and I couldn’t find any references to object_id in any repo in the organization, so this seems safe to me

@pcai pcai merged commit 41e5e4c into WinRb:main Jan 21, 2025
8 checks passed
@fwininger
Copy link
Contributor

Thanks @pcai

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