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

[WIP] Update Dry-Struct/Dry-Types. #446

Closed
wants to merge 7 commits into from
Closed

Conversation

tpendragon
Copy link
Collaborator

@tpendragon tpendragon commented Jul 5, 2018

This should add all the necessary back-ports to prevent having to
release a major version of Valkyrie, but still support the latest
dry-types and dry-struct.

Fixes #434

@tpendragon tpendragon changed the title Update Dry-Struct/Dry-Types. [WIP] Update Dry-Struct/Dry-Types. Jul 5, 2018
@tpendragon
Copy link
Collaborator Author

tpendragon commented Jul 5, 2018

I just ran it against Figgy and it all passes, but there are a couple gotchas that I sooort of feel like I either have to find a way around or release 2.0. Curious about opinions:

  1. Instance variables are not set anymore. So code depending on, IE, @title, doesn't work. Would have to get changed to attributes[:title]
  2. attributes doesn't change when you define the getters for methods anymore (part of why @title doesn't work). It's just an internally managed hash - so if you call to_h and have overridden the title method, the title in to_h will be the non-overridden version.

@tpendragon
Copy link
Collaborator Author

Part of me feels like both of these issues are undocumented and happenstance quirks of the implementation of the features of the system, and overriding title actually goes against the goals of the project.

The other part feels like this was something a real project has done and I should respect that with a patch to maintain that implementation detail.

This should add all the necessary back-ports to prevent having to
release a major version of Valkyrie, but still support the latest
dry-types and dry-struct.
This is how it used to work, so should help with backwards
compatibility.
@tpendragon tpendragon closed this Jan 4, 2019
@tpendragon tpendragon deleted the update_dry_struct branch May 8, 2019 19:00
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.

None yet

1 participant