igrigorik / vimgolf

Real Vim ninjas count every keystroke - do you?
http://www.vimgolf.com/
MIT License
672 stars 65 forks source link

Upgrade to Ruby 2.6 and Rails 5.2, update bundled gems, replace deprecated gems #312

Closed filbranden closed 3 years ago

filbranden commented 3 years ago

This will allow us to move to the Heroku-20 stack (the Heroku-16 where the app is currently running is deprecated.)

I set up a tiny app instance at Heroku-20 under https://vimgolf-staging.herokuapp.com/ where you can see the result of this PR working, so I managed to successfully test it there.

Test cases from Travis-CI also all passing.

Once this is merged, I'll upgrade the main VimGolf site to Heroku-20 as well (deadline is May 1st.)

Hettomei commented 3 years ago

Hi, I can't test it, problem during bundle install with mimemagic

$ bundle i
Fetching gem metadata from https://rubygems.org/.........
Your bundle is locked to mimemagic (0.3.5), but that version could not be
found in any of the sources listed in your Gemfile. If you haven't changed
sources, that means the author of mimemagic (0.3.5) has removed it. You'll
need to update your bundle to a version other than mimemagic (0.3.5) that
hasn't been removed in order to install.

I think the root cause is discussed here https://github.com/rails/rails/issues/41750

TL;DR; we have to wait for a solution from rails team.

filbranden commented 3 years ago

Thanks for having a look, @Hettomei !

From that Rails issue:

This is no longer a breaking issue because the maintainer of mimemagic has released a compatible version called 0.3.6

So I pushed a follow up commit to my branch to upgrade mimemagic to 0.3.6. You should be able to test it now.

I also pushed this to https://vimgolf-staging.herokuapp.com/

I also upgraded omniauth to 2.0, since Codeclimate was complaining about that. It turns out version 2.0 by default only accepts auth through POST, to work around a CSRF vulnerability, but changing the code to address that has side effects... So for now I just configured it to keep accepting GET and we can work on switching to POST on a follow up (so we can have a PR just for that.)

Looks like all checks are clean here, so please take it for a spin and let's merge/publish it so we're done with the migration to the latest stack...

Cheers! Filipe

Hettomei commented 3 years ago

I don't know what happen :

checkout your new commit and it still fail.

$ bundle i
Fetching gem metadata from https://rubygems.org/.........
Your bundle is locked to mimemagic (0.3.6), but that version could not be found in any of the sources listed in your Gemfile. If you haven't changed sources, that means the author of mimemagic (0.3.6) has removed it. You'll need to update your bundle to a
version other than mimemagic (0.3.6) that hasn't been removed in order to install.

But just :

bundle update
$ git diff

diff --git a/Gemfile.lock b/Gemfile.lock
index 5d7757a..94436a4 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -74,7 +74,7 @@ GEM
     concurrent-ruby (1.1.8)
     crass (1.0.6)
     dalli (2.7.11)
-    debug_inspector (1.0.0)
+    debug_inspector (1.1.0)
     diff-lcs (1.4.4)
     docile (1.1.5)
     erubi (1.10.0)
@@ -117,7 +117,9 @@ GEM
       mimemagic (~> 0.3.2)
     memcachier (0.0.2)
     method_source (1.0.0)
-    mimemagic (0.3.6)
+    mimemagic (0.3.9)
+      nokogiri (~> 1)
+      rake

and it works great (modulo lot of omniauth warning, but you already know).

I didn't tested the communication between the vimgolf cli and localwebsite and I don t know how.

https://vimgolf-staging.herokuapp.com seems to works the same has http://www.vimgolf.com

merge and deploy :)

filbranden commented 3 years ago

I don't know what happen :

checkout your new commit and it still fail.

Yeah, looks like they're releasing new versions and yanking old ones... https://rubygems.org/gems/mimemagic/versions

I updated the commit to ship 0.3.10, which is the latest one as of now.

and it works great (modulo lot of omniauth warning, but you already know).

There's actually a config setting that can disable those warnings... But I thought it would be best to keep them loud at the moment so we have a bit more incentive to actually address them shortly.

I didn't tested the communication between the vimgolf cli and localwebsite and I don t know how.

You can do that with:

$ GOLFHOST=http://vimgolf-staging.herokuapp.com vimgolf put ...

I just checked that it works.

(For some reason, it doesn't work with HTTPS, even with the official website, which presumably has a better certificate perhaps? Will look into it at some point.)

merge and deploy :)

Will do! Thanks for the code review.

filbranden commented 3 years ago

This has now been deployed to Heroku and, in the process, upgraded to use stack Heroku-20.

Website is working as expected, vimgolf client working well too.

igrigorik commented 3 years ago

👏👏👏

Hettomei commented 3 years ago

Thank you !