Skip to content

WIP: Rails 5

username-removed-386624 requested to merge rails-5 into master

I’ve been tracking upgrading GitLab to Rails 5 for a few months now, see #14286 (moved) for details on that.

Accomplishes most of #14286 (moved).

Isn’t it a bit early to be opening an MR for Rails 5?

Probably, but I just couldn’t resist that sweet !5555 (closed).

What’s blocking this right now?

  • Deprecation warnings, although I'd like to fix most of those separately from this MR.
  • Test failures! Lots of test failures still! If you want to go spelunking around the test failures and see if you can help that'd be greatly appreciated.
  • Fully dropping Ruby 2.1.x support.
  • I’d probably like to wait until Rails 5.0.1 comes out to actually merge this MR, since there are some bugs that may mess things up if we don’t wait for it.
  • Backporting changes which are backport-able (e.g. changing the sort methods to use a different name should be backported).
  • google-api-client can't be updated until fog-google is updated, see this PR on GitHub.
  • There's a bug in carrierwave that's blocking us from upgrading that gem, see this issue on GitHub.
  • Once the above gems are fixed up, we need to fix some issues with custom database code we have, see this comment for more info.

Deprecation warnings

See https://gitlab.com/gitlab-org/gitlab-ce/issues/14286#note_12833875 for a semi-complete list of deprecations that impact us in Rails 5.

The biggest ones are alias_method_chain and keyword arguments in controller tests.

Thankfully the latter can be fixed by this gem, which I’ve confirmed works on our codebase 👍

Errors

  • A few hundred errors in the test suite are from #20363 (moved). Resolved.
  • For whatever reason passing arguments to .sort is broken. Not sure why that happens, I suspect it’s something to do with our Sorting Helper. Resolved.

Why upgrade to Rails 5?

Companies like Heroku, GitHub, Code School, and others have written at length about getting stuck on older versions of Rails. I’d like to avoid being stuck on 4.2 forever, and the general consensus seems to be that Rails 5 is easier to upgrade to than previous major releases of Rails.

There were lots of good performance improvements in Rails 5, though without extensive profiling we can’t say if this will actually be a net benefit, as there were definitely regressions introduced at the same time.

See the Release Notes for details on everything that’s changed. For highlights on new features, improvements, etc. Big Binary has been doing an excellent job cataloguing changes for almost a year now in their Rails 5 series.

Out of scope

  • Due to how big the controller spec keyword arguments change is, I think we should apply the rails5-spec-converter in a follow-up merge request. Based on a test, it changes more than 90(!) files. This would quickly become merge conflict hell if we tried to include it here. Once this is merged, that change should come almost immediately after, otherwise the test suite will be a mess of deprecation warnings.
  • Fixing every last deprecation warning. We should fix as many as is reasonably possible, but I don’t think a few deprecation warnings should block this MR completely. I am tracking all the deprecation warnings I find, and I do intend to fix all of them, but probably not all-at-once.

“Backporting” changes

If a change can be made on master that prevents errors in the Rails 5 branch while not causing problems on Rails 4.2, we should “backport” that change to master. We want to keep this Merge Request as small as possible!

Does this MR meet the acceptance criteria?

Merge request reports