Skip to content

Support for rb_io_mode_t. #129

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

Merged
merged 1 commit into from
Apr 15, 2025
Merged

Support for rb_io_mode_t. #129

merged 1 commit into from
Apr 15, 2025

Conversation

ioquatix
Copy link
Member

Companion PR to ruby/ruby#7894.

Copy link

@Copilot 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.

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

Comments suppressed due to low confidence (2)

ext/stringio/stringio.c:39

  • Verify that the fallback definition of rb_io_mode_t as an int is consistent with its definition in 'ruby/io.h' to avoid potential type mismatches.
typedef int rb_io_mode_t;

ext/stringio/stringio.c:47

  • Ensure that all usages of the 'flags' field align with rb_io_mode_t semantics, especially regarding potential signedness differences.
rb_io_mode_t flags;

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

Should we release this soon?

@kou kou merged commit 2d3988e into master Apr 15, 2025
102 checks passed
@kou kou deleted the support-rb_io_mode_t branch April 15, 2025 11:15
matzbot pushed a commit to ruby/ruby that referenced this pull request Apr 15, 2025
@ioquatix
Copy link
Member Author

There is another fix for frozen string literals that we should merge before a release, but yes that would be great.

#128

@kou
Copy link
Member

kou commented Apr 15, 2025

OK. Let's release a new version after #128. (I'll do it in Matz-yama.)

It seems that #128 needs more discussions before we merge it.

@ioquatix
Copy link
Member Author

Yes more discussion is required but it’s definitely a big. I trust Jeremy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants