search

samvera / hydra-head

Samvera Repository Rails Engine
Other
98 stars 41 forks source link

Add support for rails 5.2 #512

Closed ebenenglish closed 4 years ago

ebenenglish commented 4 years ago

Re-submitting closed PR #470, which adds Rails 5 support to the 8.2 branch.

@samvera/hydra-head

cjcolvar commented 4 years ago

We'll have to ignore the circleci build error since it isn't configured in this branch.

ebenenglish commented 4 years ago

How much do we care about the Travis build errors?

cjcolvar commented 4 years ago

How much do we care about the Travis build errors?

I think we do care. I'm not sure how you got it to work before because it looks like the version of blacklight required by hydra-access-controls isn't compatible with rails 5.2:

hydra-head was resolved to 8.2.0, which depends on
      hydra-access-controls (= 8.2.0) was resolved to 8.2.0, which depends on
        blacklight (~> 5.10) was resolved to 5.19.2, which depends on
          rails (>= 3.2.6, < 5)

The Rails 4.2 builds are failing to connect to fedora so that's a different issue that is probably travis specific.

ebenenglish commented 4 years ago

Looks like we're using our own fork of hydra-access-controls that was created expressly for the purpose of supporting an updated rails/blacklight dependency. So "getting it to work" maybe be a bigger lift than I originally realized.

I'm seeing similar failures related to Fedora connection issues on a fresh build of the current 8-2-stable branch, so I guess there's some consistency there.

I'll try and find some time to work on this and see if I can get things passing, but it may take a couple of days and at least several more commits. Thanks!

cjcolvar commented 4 years ago

@ebenenglish Before doing anything more maybe you should see how many others are needing this. If all you need is the support-rails5 branch then it could be protected for now (or you could pin or fork). I just don't want you to spend too much of your time on this if you have a solution and no one else is needing it.

ebenenglish commented 4 years ago

@cjcolvar That's a good point -- this may not be valuable enough to the community at large to justify the effort needed. The 8-2-stable branch hasn't has any commits since December 2018 so I'm guessing this isn't a big priority for other folks. I've already forked the restored branch and gotten it up and running on my end, so this isn't blocking anything for me at the moment. I'm going to close this for now. Thanks for your help!