varvet / godmin

Admin framework for Rails 5+
http://godmin-sandbox.herokuapp.com
MIT License
486 stars 47 forks source link

Support Rails 6 #247

Closed rubendinho closed 4 years ago

rubendinho commented 5 years ago

Are there plans to add support for Rails 6?

jensljungblad commented 5 years ago

What would be needed? Feel free to create a list of what needs fixing, or start a PR if you have the time :)

rubendinho commented 5 years ago

https://github.com/varvet/godmin/blob/67dc96cf48a02663b02b8962627b39527b91a920/godmin.gemspec#L29

It looks like the gem currently requires rails to be under v6. I was not able to upgrade my app to Rails 6 with Godmin in my gemfile.

Any tips on how to override this and test things out?

jensljungblad commented 5 years ago

You can clone godmin, then point to that directory in your Gemfile:

gem "godmin", path: "path/to/cloned/godmin"

Once you've done that, you can change godmin's gemspec and remove that constraint.

@Linuus I don't remember why we have this constraint in the first place.. Would it be better to just remove the upper constraint?

Linuus commented 5 years ago

@jensljungblad I think it's mainly a safe guard to not accidentally allow major changes.

jensljungblad commented 5 years ago

@rubendinho I don't think I will have time to look into this any time soon, but if you (or perhaps someone at Varvet, @johanhalse) wants to create a PR, please feel free. We also make use of https://github.com/thoughtbot/appraisal, so hopefully it's pretty easy to run the test suite for Rails 6 as well.

johanhalse commented 5 years ago

The test suite is easy enough to run, but something about Resolvers seems to have changed between 5.2 and 6. I've done some initial digging and the find_templates function in resolver.rb isn't being run when it's supposed to in Rails 6. Documentation there isn't great and I can't find anything in the changelog.

Linuus commented 5 years ago

Yeah... it looks like they’ve changed the resolver stuff a bit. Ugh... I have nightmares about these resolvers 😆

jensljungblad commented 5 years ago

Oh man, yeah that sucks. It was always the most annoying part of Godmin :/

johanhalse commented 5 years ago

Good news though, I have a PR to fix it! https://github.com/varvet/godmin/pull/248

The tests all pass but are there any obvious red flags? I haven't worked on the Godmin resolvers before so I'm not super confident.

jensljungblad commented 5 years ago

Hm, I don’t think so. If the tests pass and the dummy app looks okay, I’d say that’s good enough. You can run the dummy app with test/dummy/bin/rails s if you want to poke around.

mbajur commented 4 years ago

Any chances to publish a new gem version with rails 6 support anytime soon? :)

EDIT: It turned out godmin is not yet ready for rails 6, i'm gonna try to prepare it on a PR #250 and let you know

mbajur commented 4 years ago

Ok, PR ready for a review and merge ✅

jensljungblad commented 4 years ago

@mbajur Merged. Feel free to try it out for a bit. If everything seems fine we can close this issue and release a new version.

Thanks!

mbajur commented 4 years ago

@jensljungblad i am actually actively using my branch for last 2 days and it seems to be working perfectly fine. Consider it as a confirmation everything is good with rails 6 :)

jensljungblad commented 4 years ago

Looks like @johanhalse released it! 🎉