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

Rails 5 #242

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Rails 5 #242

wants to merge 5 commits into from

Conversation

pakallis
Copy link
Member

@pakallis pakallis commented Sep 28, 2016

http://guides.rubyonrails.org/5_0_release_notes.html

Things pending

  • Check if it works in production env
  • Fix forbidden(403) in test_runs_controller_test.rb
  • Fix feature tests
  • Tests appear to be slow

@ispyropoulos
Copy link
Member

  • Is there something forcing us to upgrade? e.g. unsupported gems for 4.2
  • Do we gain any significant benefits from the upgrade at this point? e.g. faster ActiveRecord, smaller memory footprint, less objects in memory, etc.

Bare in mind that we don't have a staging environment. We need to test this in staging, with staging workers, webhooks, database etc. It seems like an opportunity to set this up.

@pakallis
Copy link
Member Author

pakallis commented Sep 29, 2016

  • We gain access to the ActionController::API, so that our API has no unecessary middleware(e.g. SessionMiddleware). This might make the API faster
  • We gain access to the ActionCable which might be a solution for live updates - We have to figure out if it is worth it.
  • We now have cleaner syntax for controller tests(E.g. instead of post :create, {id: 2} we have the more explicit post :create, params: {id: 3}.
  • We gain access to the redirect_back helper that has a fallback location when the referrer doesn't exist

Apart from that, there is nothing that forces us to upgrade

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