Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the SameSite cookie attribute by renaming the Empty variant to None, aligning it with the underlying Rust cookie crate's enum and standard cookie semantics.
Key Changes:
- Renamed
SameSite::EmptytoSameSite::Nonein the Rust enum definition - Updated constant registration to expose
SameSite::Noneinstead ofSameSite::Empty - Updated Ruby stub documentation to reflect the
Noneconstant
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/cookie.rs | Updated define_ruby_enum! macro invocation to use None variant instead of tuple mapping (Empty, None), and updated constant registration from Empty to None |
| lib/wreq_ruby/cookie.rb | Renamed Ruby constant from Empty to None and updated associated documentation comment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ruby does not have a None type like Python.
Therefore, creating an alias that maps SameSite::Empty to None is unnecessary and may even cause confusion, since Ruby conventions typically use nil to represent the absence of a value.
To keep the API consistent with Ruby’s type system and avoid implying Python-style semantics, the alias has been removed / corrected accordingly.