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

Deprecate PowerConverter #4097

Merged
merged 7 commits into from Oct 19, 2019
Merged

Deprecate PowerConverter #4097

merged 7 commits into from Oct 19, 2019

Conversation

no-reply
Copy link
Member

@no-reply no-reply commented Oct 18, 2019

PowerConverter is a community-maintained gem, but is not in the Samvera
namespace. Depending on it under these circumstances is not in keeping with our
policy.

Rather than simply inline the PowerConverter class, this proposes pushing in
the direction a more generic ruby pattern that inspired
it. http://devblog.avdi.org/2012/05/07/a-ruby-conversion-idiom/

  • When casting to a specific class, use MyClass(thing_to_cast_from), as is done in Ruby core.
  • When the conversion is polymorphic, use a .*_for method as a factory

Refs #4096

@samvera/hyrax-code-reviewers

Tom Johnson added 2 commits October 18, 2019 11:57
`PowerConverter` is a community-maintained gem, but is not in the Samvera
namespace. Depending on it under these circumstances is not in keeping with our
policy.

Rather than simply inline the `PowerConverter` class, this proposes pushing in
the direction a more generic ruby pattern that inspired
it. http://devblog.avdi.org/2012/05/07/a-ruby-conversion-idiom/
Allow `WorkflowAction` to handle casting to an action name, using `.name_for(object_to_cast)`.
@no-reply no-reply changed the title Deprecate PowerConverter based to_sipity_action Deprecate PowerConverter Oct 18, 2019
@no-reply no-reply force-pushed the deprecate-pc branch 3 times, most recently from f4c1ef3 to cba1ce5 Compare October 18, 2019 21:34
Tom Johnson added 5 commits October 18, 2019 16:42
This was very sparsely used. Instead of porting it elsewhere, just cast the one
caller individually.

If folks are using this downstream (I'd guess they aren't) we could consider
putting this mapper at the top `Hyrax` level, or somewhere similar.
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

2 participants