Finalize rails 6 update #594

BatedUrGonnaDie · 2019-08-30T02:53:00Z

Updates a ton of gems and packages. Just did blanket updates for both yarn and bundle.

Closes #517

Also, is it possible to highjack the beta branch to make sure that the cookie changes will not break production? There was 2 this time and the extra assurance would be nice.

BatedUrGonnaDie · 2019-08-30T02:53:34Z

Gemfile

@@ -45,7 +45,7 @@ group :production do
 end

 # administration
-gem 'administrate'
+gem 'administrate', github: 'thoughtbot/administrate'


Administrate doesn't have a release that will bundle install when using rails 6, but master does work. They have a plan to release a version sometime soon to fix this.

BatedUrGonnaDie · 2019-08-30T02:54:01Z

app/blueprints/api/v3/category_blueprint.rb

@@ -1,5 +1,5 @@
 class Api::V3::CategoryBlueprint < Blueprinter::Base
  fields :id, :name, :created_at, :updated_at

-  association :game, blueprint: Api::V3::GameBlueprint, if: ->(_, options) { options[:toplevel] == :category }
+  association :game, blueprint: Api::V3::GameBlueprint, if: ->(_, _, options) { options[:toplevel] == :category }


Blueprinter has a depreciation warning now, because they changed the conditionals to take 3 parameters now instead of 2.

BatedUrGonnaDie · 2019-08-30T02:54:57Z

app/controllers/concerns/authenticatable.rb

  def sign_in(user)
-    self.current_user = user
+    create_auth_session(user)


When updating authie, I noticed now that it recommends using this method over calling self.current_user = user

BatedUrGonnaDie · 2019-08-30T02:55:25Z

app/controllers/runs_controller.rb

@@ -1,7 +1,5 @@
 require 'uri'

-require 'speedrundotcom'


Not sure if this was no longer needed because of the new autoloader, or because at some point /lib was added to the autoloading path.

BatedUrGonnaDie · 2019-08-30T02:56:11Z

app/models/concerns/speedrun_dot_com_run.rb

-require 'speedrundotcom'
-
-module SRDCRun
+module SpeedrunDotComRun


The new autoloader didn't like this old class name, so I updated it to be consistent with all the models that reference speedrun.com stuff.

BatedUrGonnaDie · 2019-08-30T02:57:40Z

lib/parser/livesplit_core_parser.rb

 end

-class Parser
+class Parser::LivesplitCoreParser


The new autoloader also did not like the way this was defined before. Since /lib is the top level namespace, anything under it needs to be namespaced according to its directory structure.

BatedUrGonnaDie · 2019-08-30T02:58:28Z

spec/controllers/api/v4/runs_controller_spec.rb

@@ -175,7 +175,7 @@
      it 'returns the correct content-type header' do
        request.headers['Accept'] = 'application/wsplit'

-        expect(response.content_type).to eq('application/wsplit')
+        expect(response.media_type).to eq('application/wsplit')


response.content_type now returns the full content type header as is, while response.media_type now returns just the MIME component as previous versions of rails did.

glacials

lgtm! Thanks for the comments describing the reason for each change. Yeah feel free to take over beta, just delete the current branch or do whatever you need to do if it's still leftover from racing.

BatedUrGonnaDie · 2019-09-01T01:55:04Z

I totally forgot that in development mode it was only loading classes that were accessed in a request. When I went to test on beta I hit a autoloading error, and this is where I ended up. Since I had to change a lot of names and removed some stuff, another review seems appropriate.

BatedUrGonnaDie · 2019-09-01T01:56:57Z

lib/rails_extensions/read_only.rb

@@ -1,7 +0,0 @@
-if Rails.configuration.read_only
-  module ActiveRecord::Base
-    def readonly?


I couldn't find any reference to this method outside of the definition. Looks like everything was just checking the environment variable.

BatedUrGonnaDie · 2019-09-01T01:57:35Z

lib/parser/livesplit_core_version

Since I needed LSC to be in the Parser:: namespace, I took this opportunity to just update it as well.

BatedUrGonnaDie · 2019-09-01T01:58:55Z

lib/programs/exchange_format.rb

@@ -1,4 +1,4 @@
-module ExchangeFormat
+module Programs::ExchangeFormat


Since lib/ is now a top level namespace, zeitwerk is expecting programs/ to define a new namespace. All programs now reside in the namespace, and allows us to easily get all constants of this namespace in run.rb instead of needing to manually name all programs there every time we change something.

👍 Love it!

BatedUrGonnaDie · 2019-09-01T01:59:54Z

lib/assets/bower_components/bootstrap-toggle/.bower.json

@@ -1,41 +0,0 @@
-{


I'm pretty positive we don't use bower anymore, and it was causing an issue for some reason (not really sure why). Seemed like a good time to kill this folder.

BatedUrGonnaDie · 2019-09-01T02:00:33Z

config/puma.rb

@@ -16,6 +16,9 @@
 #
 environment ENV.fetch("RAILS_ENV") { "development" }

+# Specifies the `pidfile` that Puma will use.
+pidfile ENV.fetch("PIDFILE") { "tmp/pids/server.pid" }


I found this from Rails diff which is a pretty cool website for checking config changes between rails versions.

BatedUrGonnaDie · 2019-09-01T02:01:39Z

config/initializers/inflections.rb

@@ -14,3 +14,8 @@
 # ActiveSupport::Inflector.inflections(:en) do |inflect|
 #   inflect.acronym 'RESTful'
 # end
+
+ActiveSupport::Inflector.inflections(:en) do |inflect|
+  inflect.acronym('SplitterZ')


It didn't make sense to change the file name for these 2 programs to me, since logically they couldn't be considered words. Instead by making them an inflector zeitwerk will use this definied camelization instead of needing the underscores. I can do this for any of the other programs if you think that would be a better idea than renaming the files

Can we add a comment to each of these programs' files explaining that? I'm afraid we'll add a program later that needs this, forget about it, and be confused for hours or days about why it doesn't work the same way the others do.

I can totally see that happening as well, will do.

glacials

SeemsGood

glacials · 2019-09-01T03:36:02Z

lib/programs/exchange_format.rb

@@ -1,4 +1,4 @@
-module ExchangeFormat
+module Programs::ExchangeFormat


👍 Love it!

glacials · 2019-09-01T03:41:10Z

config/initializers/inflections.rb

@@ -14,3 +14,8 @@
 # ActiveSupport::Inflector.inflections(:en) do |inflect|
 #   inflect.acronym 'RESTful'
 # end
+
+ActiveSupport::Inflector.inflections(:en) do |inflect|
+  inflect.acronym('SplitterZ')


Can we add a comment to each of these programs' files explaining that? I'm afraid we'll add a program later that needs this, forget about it, and be confused for hours or days about why it doesn't work the same way the others do.

glacials · 2019-09-01T03:48:12Z

lib/tasks/livesplit_core.rake

+  data.sub!('module LiveSplitCore', 'module Parser::LiveSplitCore')
+  File.open("#{PARSER_FOLDER}/LiveSplitCore.rb", 'w') do |f|
+    f.write(data)
+  end


Another possible solution to this problem in case you haven't seen it, though I have no opinion either way:

https://stackoverflow.com/a/8762740

Thanks for the link, I ended up liking that way a bit more 👍

Finalize rails 6 update

a179489

BatedUrGonnaDie requested a review from glacials August 30, 2019 02:53

BatedUrGonnaDie commented Aug 30, 2019

View reviewed changes

glacials approved these changes Aug 30, 2019

View reviewed changes

Fix a ton of autoloading issues

6aa3bf6

BatedUrGonnaDie requested a review from glacials September 1, 2019 01:53

BatedUrGonnaDie commented Sep 1, 2019

View reviewed changes

glacials approved these changes Sep 1, 2019

View reviewed changes

PR comments

6f269fa

BatedUrGonnaDie merged commit 0ba738f into master Sep 1, 2019

BatedUrGonnaDie deleted the rails-6 branch September 1, 2019 05:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Finalize rails 6 update #594

Finalize rails 6 update #594

BatedUrGonnaDie commented Aug 30, 2019 •

edited

Loading

BatedUrGonnaDie Aug 30, 2019

BatedUrGonnaDie Aug 30, 2019

BatedUrGonnaDie Aug 30, 2019

BatedUrGonnaDie Aug 30, 2019

BatedUrGonnaDie Aug 30, 2019

BatedUrGonnaDie Aug 30, 2019

BatedUrGonnaDie Aug 30, 2019

glacials left a comment

BatedUrGonnaDie commented Sep 1, 2019

BatedUrGonnaDie Sep 1, 2019

BatedUrGonnaDie Sep 1, 2019

BatedUrGonnaDie Sep 1, 2019

glacials Sep 1, 2019

BatedUrGonnaDie Sep 1, 2019

BatedUrGonnaDie Sep 1, 2019

BatedUrGonnaDie Sep 1, 2019

glacials Sep 1, 2019

BatedUrGonnaDie Sep 1, 2019

glacials left a comment

glacials Sep 1, 2019

glacials Sep 1, 2019

glacials Sep 1, 2019

BatedUrGonnaDie Sep 1, 2019

		@@ -1,4 +1,4 @@
		module ExchangeFormat
		module Programs::ExchangeFormat

Finalize rails 6 update #594

Finalize rails 6 update #594

Conversation

BatedUrGonnaDie commented Aug 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glacials left a comment

Choose a reason for hiding this comment

BatedUrGonnaDie commented Sep 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glacials left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BatedUrGonnaDie commented Aug 30, 2019 •

edited

Loading