Skip to content

Conversation

@nevans
Copy link
Collaborator

@nevansnevans commented Nov 9, 2024

For new data structs, I'd prefer frozen by default and I don't want to support the entire Struct API. Data is almost exactly what I want, but it's not available until ruby 3.2.

So this adds a DataLite class that closely matches ruby 3.2's Data class, and it can be a drop-in replacement for Data. Net::IMAP::Data is an alias for Net::IMAP::DataLite, so when we remove our implementation, the constant will resolve to ruby's ::Data.

Ideally, we wouldn't define this on newer ruby versions at all, but that breaks the YAML serialization for our test fixtures.

I originally implemented this by hand, with my own tests and partially by looking at ruby's struct.c. But I later copied all of the tests (and some of the implementation) from the polyfill-data gem. I've updated the tests so that they use Data as it is resolved inside the Net::IMAP namespace. Copyright notices have been added to the appropriate files to satisfy the MIT license terms.


This is a prerequisite for the following PRs (as currently implemented):

@nevans
Copy link
CollaboratorAuthor

@shugo what do you think about adding this, temporarily. We'd remove it with v0.6.0, when we raise the minimum required ruby to 3.2.

I actually started with PORO classes, but then I needed to implement the same #==, #eql?, #hash, #deconstruct, #deconstruct_keys multiple times.. at which point I wound up with this!

Alternatively, I could just use Struct as we've been doing. But I'd rather not need to support Struct's larger API surface.

@nevans
Copy link
CollaboratorAuthor

nevans commented Nov 9, 2024

@saturnflyer I'm tagging you too, just so you know that I've copied your tests with very little modification. I'd already started down the path of my own implementation and I kept some of that, but I copied a bunch of your implementation too. Honestly, I should probably amend the commit to add you as Co-authored-by. 🙂

@nevansnevansforce-pushed the data-polyfill branch 3 times, most recently from 90d5b85 to 2a4312fCompareNovember 11, 2024 16:03
@nevansnevansforce-pushed the data-polyfill branch 4 times, most recently from aa279bd to 15c1692CompareNovember 25, 2024 17:09
For new data structs, I don't want to commit to supporting the entire Struct API and I'd prefer frozen by default. `Data` is exactly what I want but it's not available until ruby 3.2. So this adds a DataLite class that closely matches ruby 3.2's Data class and can be a drop-in replacement for Data. Net::IMAP::Data is an alias for Net::IMAP::DataLite, so when we remove this implementation, the constant will resolve to ruby's ::Data. The most noticable incompatibility is that member names must be valid local variable names. Ideally, we wouldn't define this on newer ruby versions at all, but that breaks the YAML serialization for our test fixtures. So, on newer versions of ruby, this class inherits from `Data` and only reimplements the two methods that are needed for YAML serialization. This serves the additional purpose of ensuring that the same tests pass for both the core `Data` class and our reimplementation. Most of the test code and some of the implementation code has been copied from the polyfill-data gem and updated so that they use "Data" as it is resolved inside the "Net::IMAP" namespace. Copyright notices have been added to the appropriate files to satisfy the MIT license terms. Co-authored-by: Jim Gay <[email protected]>
@nevansnevans merged commit c287366 into ruby:masterDec 14, 2024
13 checks passed
@nevansnevans deleted the data-polyfill branch December 14, 2024 15:24
@nobu
Copy link
Member

nobu commented Jan 2, 2026

Since #538 dropped support for ruby 3.1, this will be no longer needed.

Sign up for freeto 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

@nevans@nobu