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

RFC9586 UIDONLY support #366

Merged
merged 16 commits into from
Jan 3, 2025
Merged

RFC9586 UIDONLY support #366

merged 16 commits into from
Jan 3, 2025

Conversation

avdi
Copy link
Contributor

@avdi avdi commented Dec 17, 2024

AKA "UI donly", apparently.

avdi and others added 8 commits January 3, 2025 09:57
This is accomplished by making UIDFetchData subclass FetchData.  I
instinctively prefer to extact a module or a shared superclass and put
all of the accessor methods on that.  I did it this way in order to
preserve the attribute accessor rdoc on FetchData.

But it feels wrong to me that UIDFetchData is subclassing FetchData:
* It's a small violation of Liskov's Substitution Principle, for #seqno
  (and, to a lesser extent, #uid).
* Any case statements or pattern matching that are looking for FetchData
  will also match UIDFetchData, which may not be correct.  It also
  complicates matching only UIDFetchData, because you need to ensure
  that when/in UIDFetchData comes before any when/in FetchData.

On the other hand, it's probably a good thing that FetchData#=== matches
UIDFetchData too.  Other than #seqno (which is prohibited by `UIDONLY`)
any code that works for FetchData today should work with UIDFetchData.
@nevans nevans force-pushed the RFC9586-UIDONLY branch 2 times, most recently from dcbef82 to 57ac8b4 Compare January 3, 2025 20:56
nevans added 7 commits January 3, 2025 16:24
This way, we can get the correct Struct field names for both classes,
without needing to undef, alias, etc.  It also fixes Struct methods like
`to_h` and `deconstruct_keys`.
This reduces duplication between `fetch_internal` and `store_internal`.
@nevans nevans added the enhancement New feature or request label Jan 3, 2025
@nevans nevans merged commit 2fdc80f into ruby:master Jan 3, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants