Move addressable to gemspec

[jrochkind]

Addressable gem is used by actual runtime code! It needs to be in the gemspec. Otherwise if an app using active_encode doesn't already have addressable as it's own dependency for other reasons -- when the active_encode logic that uses addressable is reached, it'll error!

I am not sure if other things listed in the Gemfile may also be in this state -- I think some other things listed in Gemfile may not actually be used at all anymore, like "shingoncoder"? I could find no reference to that one in codebase. But didn't touch it here.

There is also the issue of all the adapter-specific dependencies that aren't listed as dependencies... But we're just dealing with addressable here

[cjcolvar]

I think I did it this way with the adapter-specific dependencies because I didn't know how to make them optional since we shouldn't need to require all of the adapter-specific dependencies when someone is usually only going to use one of them. Valkyrie might have a better pattern that could be followed for this.

We should be able to drop shingoncoder as it was an experiment by Justin Coyne to have something akin to the ffmpeg adapter that now exists in active_encode. Similarly I think the zencoder adapter could be removed. It was an inspiration for the adapter api but has never really been used and is probably way out of date. The matterhorn adapter was the original one which allowed avalon to migrate off of matterhorn but hasn't been used for a few years so it could probably be removed as well. That would go a good ways to cleaning things up and cutting out adapter-specific dependencies that aren't getting updated.

[jrochkind]

If Shingongocer and zencoder aren't referneced at all in teh current codebase, that's straightforward enough, no reason to have them included in the gemfile.

There isn't any great pattern for "optional" dependencies. I haven't seen anything from valkyrie on it.

Rails does set some precedent for such things, with how for instance neither pg nor mysql are actual Rails dependencies, but you need one or the other if you're using the respective database. It gets a bit messy.

I wrote up my analysis here: https://bibwild.wordpress.com/2015/09/09/optional-gem-dependencies/

So anyway, right now, an app using active_encode is responsible for adding those "optional" dependencies to it's Gemfile. But it's not easy to figure out what they are. For instance, in trying to use the MediaConvert adapter, I had to go through a process of "okay, it's throwing an error this class isn't present. Oh, I guess there's a gem I'm supposed to add to my local app? Which one? Add it!" -- iterate three times to get three "optional" dependencies.

As a first step, I'd say the inline comment docs for each adapter ought to list the dependencies (and major version expected?) that an app has to include to use them. Plus examples in README. Right now they do not.

[tpendragon]

Valkyrie does something very similar to Rails for optional dependencies: https://github.com/samvera/valkyrie/blob/1ad73aed5b41aa1957912f198015629e05a2a2b1/lib/valkyrie/persistence/fedora.rb#L3-L9

[jrochkind]

Thanks @tpendragon . That example you link to is willing to take any version of ldp.

While this is what Rails does too... it's a mess, even with Rails. And Rails tries to get around the mess by generating a more specific version requirement into the generated Gemfile, which valkyrie or active_encode isn't going to do I don't think. And it's still a mess in Rails.

I'd recommend adding at least a major version requirement there (assuming ldp in this example does semver). You can just say gem 'ldp', "~> 1.0", and it should work just fine, and raise if the hosting app tries to use an ldp version that isn't "~> 1.0", just as if it doesn't have ldp at all. And add that to the error message to to make things more clear to the user.

I think that's a significant improvement on what Rails does with no real downside -- yes, you'd have to release a new valkyrie/active_encode if you wanted to change what veression of the "optional" dependency you want to support, but that just makes it more similar to "real" dependencies, I think that's a plus not a downside.