Upgrade to Ruby 2.7.6 #953

nimmolo · 2022-05-27T21:08:51Z

Steps to try the update, a couple safeguards to prevent a glitch updating the gem mini-racer to 0.6.2.
These can be performed in vagrant console:

Add the new Ruby version alongside 2.6.6

rvm install 2.7.6
/bin/bash --login
rvm use 2.7.6
ruby -v (check!)

Update rubygems and bundler

gem update --system
gem update bundler

Delete your gemfile.lock and update the gems

bundle update

coveralls · 2022-05-27T21:21:22Z

Coverage remained the same at 93.065% when pulling 008ebfa on ruby-2.7 into 2c7eabc on master.

nimmolo · 2022-05-27T21:22:43Z

Incredible. Nothing but net!

There are many notices of the upcoming kwarg behavior change in our code to fix, but they're just deprecation warnings for now, and only caused by very few methods. cure_acts_as_versioned needs a tweak too.

If I can track the methods down, I'll fix them in this PR.

pellaea · 2022-05-27T21:29:18Z

Excellent news! Yeah, I've slowly been fixing the keyword arguments whenever I come across them. But I imagine there are still quite a few left. I sure hope we can keep acts_as_versioned around. That's a hard one to switch out. We ran up against that a few years ago, and were very thankful cure_acts_as_versioned came to the rescue!

…

On Fri, May 27, 2022 at 5:22 PM andrew nimmo ***@***.***> wrote: Incredible. Nothing but net! There are many notices of the upcoming kwarg behavior change in our code to fix, but they're just deprecation warnings for now, and only caused by very few methods. cure_acts_as_versioned needs a tweak too. If I can track them down, I'll fix them in this PR. — Reply to this email directly, view it on GitHub <#953 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYTNNNUEII4HFUJWVDSQDTVME4K5ANCNFSM5XFPEI5Q> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

nimmolo · 2022-05-27T21:37:07Z

I guess you have been fixing them — It's only one method!!!
NameTest#do_name_parse_test(str, expects, deprecated: false)
How can I fix? I've been reading up but i'm not sure I understand where to fix it - in the method, or in the calls to the method.
I have all the line numbers that call it in name_test.rb

I also think cure_acts_as_versioned is fixable, it's our fork isn't it?

nimmolo · 2022-05-27T21:54:13Z

In acts_as_versioned.rb, it's here on line 160:

        self.version_association_options  = {
                                                    :class_name  => "#{self.to_s}::#{versioned_class_name}",
                                                    :foreign_key => versioned_foreign_key,
        }.merge(options[:association_options] || {})

and then used here on line 243:

        included do
          has_many :versions, self.version_association_options

Should we splat has_many :versions, **self.version_association_options?

What I don't understand is what the new usage is, for kwargs. I've read a bunch of posts about it and I still don't get it.

For the NameTest#do_name_parse_test(str, expects, deprecated: false) it seems like I could move the deprecated into the expects, but that's not very DRY.

pellaea · 2022-05-27T22:00:17Z

I think I've misunderstood something. I'm going to have to look at this more and get back to you later...

…

On Fri, May 27, 2022 at 5:54 PM andrew nimmo ***@***.***> wrote: In acts_as_versioned.rb, it's here on line 160: self.version_association_options = { :class_name => "#{self.to_s}::#{versioned_class_name}", :foreign_key => versioned_foreign_key, }.merge(options[:association_options] || {}) What I don't understand is what the new usage is, for kwargs. I've read a bunch of posts about it and I still don't get it. For the NameTest#do_name_parse_test(str, expects, deprecated: false) it seems like I could move the deprecated into the expects, but that's not very DRY. — Reply to this email directly, view it on GitHub <#953 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYTNNNHE2NCX7CB5UUWL63VMFABBANCNFSM5XFPEI5Q> . You are receiving this because you commented.Message ID: ***@***.***>

JoeCohen · 2022-05-27T22:02:21Z

So much has been happening, I can't keep up.
I'm glad you've merged everything without waiting for me.

Re cure_acts_as_versioned.

It is our fork.
If it's easily fixable, it should be done.
IMO we should think about moving to a more current, more widely gem, so that we don't have to maintain it ourselves, even if that means losing all existing old versions. Example paper_trail.

pellaea · 2022-05-27T22:14:46Z

We talked about doing that -- re: using another gem, or the built-in functionality of recent versions of rails -- in a previous upgrade but it turned out to be a huge task, so we fell back on the patch. I forgot that we did it ourselves.

…

On Fri, May 27, 2022 at 6:02 PM Joseph D. Cohen ***@***.***> wrote: So much has been happening, I can't keep up. I'm glad you've merged everything without waiting for me. Re cure_acts_as_versioned. - It is our fork. - If it's easily fixable, it should be done. - IMO we should think about moving to a more current, more widely gem, so that we don't have to maintain it ourselves, even if that means losing all existing old versions. Example paper_trail <https://rubygems.org/gems/paper_trail>. — Reply to this email directly, view it on GitHub <#953 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYTNNKEWF6HQEKWJY7HIODVMFA7PANCNFSM5XFPEI5Q> . You are receiving this because you commented.Message ID: ***@***.***>

nimmolo · 2022-05-27T22:19:02Z

@JoeCohen - Thanks! I do feel like the acts_as_versioned fix would be easy, I just don't understand the syntax change. But I believe it's in the lines I pointed out. Here's the deprecation warning:

/home/vagrant/.rvm/gems/ruby-2.7.6/bundler/gems/acts_as_versioned-59a408816683/lib/acts_as_versioned.rb:243: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call

@pellaea - More likely it is I who misunderstood something! But here is a sample of the warnings:

/vagrant/mushroom-observer/test/models/name_test.rb:1086: warning: Passing the keyword argument as the last hash parameter is deprecated:19
/vagrant/mushroom-observer/test/models/name_test.rb:19: warning: The called method `do_name_parse_test' is defined here
/vagrant/mushroom-observer/test/models/name_test.rb:964: warning: Passing the keyword argument as the last hash parameter is deprecated1:18
/vagrant/mushroom-observer/test/models/name_test.rb:19: warning: The called method `do_name_parse_test' is defined here
/vagrant/mushroom-observer/test/models/name_test.rb:1116: warning: Passing the keyword argument as the last hash parameter is deprecated
/vagrant/mushroom-observer/test/models/name_test.rb:19: warning: The called method `do_name_parse_test' is defined here

It's basically each time do_name_parse_test is called. Here's the code for the test:

  # Parse a string, with detailed error message.
  def do_name_parse_test(str, expects, deprecated: false)
    ...
  end

  ...

  def test_name_parse_21
    do_name_parse_test(
      "Amanita “quoted”",
      text_name: 'Amanita "quoted"',
      real_text_name: 'Amanita "quoted"',
      search_name: 'Amanita "quoted"',
      real_search_name: 'Amanita "quoted"',
      sort_name: 'Amanita quoted"',
      display_name: '**__Amanita "quoted"__**',
      parent_name: "Amanita",
      rank: :Species,
      author: ""
    )
  end

JoeCohen · 2022-05-27T22:20:06Z

What we discovered when we talked about it was that there's no migration path from acts_as_versioned to paper_trail; We'd have to start over with the current model versions as the baseline. I personally think it's worth it in order to reduce issues of gem incompatibility. On Fri, May 27, 2022 at 3:14 PM Jason Hollinger ***@***.***> wrote:

…

We talked about doing that -- re: using another gem, or the built-in functionality of recent versions of rails -- in a previous upgrade but it turned out to be a huge task, so we fell back on the patch. I forgot that we did it ourselves. On Fri, May 27, 2022 at 6:02 PM Joseph D. Cohen ***@***.***> wrote: > So much has been happening, I can't keep up. > I'm glad you've merged everything without waiting for me. > > Re cure_acts_as_versioned. > > - It is our fork. > - If it's easily fixable, it should be done. > - IMO we should think about moving to a more current, more widely gem, > so that we don't have to maintain it ourselves, even if that means losing > all existing old versions. Example paper_trail > <https://rubygems.org/gems/paper_trail>. > > — > Reply to this email directly, view it on GitHub > < #953 (comment) >, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/AAYTNNKEWF6HQEKWJY7HIODVMFA7PANCNFSM5XFPEI5Q > > . > You are receiving this because you commented.Message ID: > ***@***.***> > — Reply to this email directly, view it on GitHub <#953 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAALDFCVBWVTBOX5HUYCWLLVMFCOBANCNFSM5XFPEI5Q> . You are receiving this because you commented.Message ID: ***@***.***>

This works!

nimmolo · 2022-05-27T23:20:43Z

I solved the NameTest kwarg issue with a small refactor.

…

On May 27, 2022, at 4:19 PM, codeclimate[bot] ***@***.***> wrote: Code Climate has analyzed commit e553f05 and detected 0 issues on this pull request. View more on Code Climate. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

nimmolo · 2022-05-28T00:15:21Z

Will put that NameTest refactor in a separate PR off master tonight. But for the time being, it helps in checking out this PR. Note that : a) this PR bundles Rails 6.1, since it's assuming that PR would get merged first. b) it is easy to switch Ruby version back to 2.6.6 after trying this. just `rvm use 2.6.6` - both versions will still be installed on your vagrant box, and 2.6.6 remains the default.

…

On May 27, 2022, at 4:19 PM, codeclimate[bot] ***@***.***> wrote: Code Climate has analyzed commit e553f05 and detected 0 issues on this pull request. View more on Code Climate. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

JoeCohen · 2022-05-28T00:45:35Z

@nimmolo. Re acts_as_versioned
I'm also not getting it.
act_as_versioned line 243:

has_many :versions, self.version_association_options

self.version_association_options is a hash
act_as_versioned lines 160-163:

self.version_association_options  = {
                                            :class_name  => "#{self.to_s}::#{versioned_class_name}",
                                            :foreign_key => versioned_foreign_key,
}.merge(options[:association_options] || {})

So the call to has_many does use a hash as its last argument.
But has_many ought to handle that, as it splats the options param:

# File activerecord/lib/active_record/associations.rb, line 1370
def has_many(name, scope = nil, **options, &extension)

I don't know what to try. Maybe add a second positional arg:

has_many :versions, nil, self.version_association_options

Maybe try an extra splat

has_many :versions, **(self.version_association_options)

Even worse, I have no idea how to try out these ideas. I forget how I worked with acts_as_versioned when I forked it.

nimmolo · 2022-05-28T05:19:47Z

Joe - I'm going to try the extra splat (forking the repo and telling Gemfile to load my local version). Will report back!

…

On May 27, 2022, at 5:45 PM, Joseph D. Cohen ***@***.***> wrote: @nimmolo. Re acts_as_versioned I'm also not getting it. act_as_versioned line 243: has_many :versions, self.version_association_options self.version_association_options is a hash act_as_versioned lines 160-163: self.version_association_options = { :class_name => "#{self.to_s}::#{versioned_class_name}", :foreign_key => versioned_foreign_key, }.merge(options[:association_options] || {}) So the call to has_many does use a hash as its last argument. But has_many ought to handle that, as it splats the options param: # File activerecord/lib/active_record/associations.rb, line 1370 def has_many(name, scope = nil, **options, &extension) I don't know what to try. Maybe add a second positional arg: has_many :versions, nil, self.version_association_options Maybe try an extra splat has_many :versions, **(self.version_association_options) Even worse, I have no idea how to try out these ideas. I forget how I worked with acts_as_versioned when I forked it. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

nimmolo · 2022-05-28T09:22:34Z

Hi Joe - It’s not necessary for my local test, but in case it works... Could you grant me commit privileges to MO’s “acts_as_versioned”? I seem to have write access to many of the repos, but not this one. nimmo

…

On May 27, 2022, 10:19 PM -0700, nimmo ***@***.***>, wrote: Joe - I'm going to try the extra splat (forking the repo and telling Gemfile to load my local version). Will report back! On May 27, 2022, at 5:45 PM, Joseph D. Cohen ***@***.***> wrote: > @nimmolo. Re acts_as_versioned > I'm also not getting it. > act_as_versioned line 243: > has_many :versions, self.version_association_options > self.version_association_options is a hash > act_as_versioned lines 160-163: > self.version_association_options = { > :class_name => "#{self.to_s}::#{versioned_class_name}", > :foreign_key => versioned_foreign_key, > }.merge(options[:association_options] || {}) > So the call to has_many does use a hash as its last argument. > But has_many ought to handle that, as it splats the options param: > # File activerecord/lib/active_record/associations.rb, line 1370 > def has_many(name, scope = nil, **options, &extension) > I don't know what to try. Maybe add a second positional arg: > has_many :versions, nil, self.version_association_options > Maybe try an extra splat > has_many :versions, **(self.version_association_options) > Even worse, I have no idea how to try out these ideas. I forget how I worked with acts_as_versioned when I forked it. > — > Reply to this email directly, view it on GitHub, or unsubscribe. > You are receiving this because you were mentioned.Message ID: ***@***.***>

Fixes "last argument as keyword parameters is deprecated" as kwargs by double-splatting the last argument. See https://www.ruby-lang.org/en/news/2019/12/25/ruby-2-7-0-released/ MushroomObserver/mushroom-observer#953 (comment)

nimmolo · 2022-05-29T23:48:28Z

I believe this is good to go:

Joe has updated cure_acts_as_versioned to fix deprecation warnings
Everything in the app is fixed to avoid deprecation warnings
...but when it's switched, or before, will the vagrantfile need an update too (in developer-startup)?

nimmolo · 2022-05-29T23:53:01Z

Actually, #957 should merge first. But then the dep warnings will be fixed.

test/controller_extensions.rb

test/controllers/account_controller_test.rb

nimmolo · 2022-05-30T00:49:55Z

Update: Both #957 and #960 should merge first, to clear Codeclimate warnings.

test/controllers/account_controller_test.rb

test/session_extensions.rb

codeclimate · 2022-05-30T09:08:18Z

Code Climate has analyzed commit 008ebfa and detected 0 issues on this pull request.

View more on Code Climate.

nimmolo · 2022-05-30T10:01:44Z

Good to merge if you're ready. Rubocop says 👍, Codeclimate says🤙

JoeCohen · 2022-10-11T08:02:35Z

I woke up thinking about this problem. 1. You now have full access (Admin) to that repo. 2. Simply splatting the arg works for me: has_many :versions, **self.version_association_options (I edited acts_as_versioned.rb on my vm.) So you can simply edit that file in the repo. And also update CHANGELOG. Or I can do both in the morning. (I'm going back to bed.) -- Joe On Sat, May 28, 2022 at 2:22 AM andrew nimmo ***@***.***> wrote:

…

Hi Joe - It’s not necessary for my local test, but in case it works... Could you grant me commit privileges to MO’s “acts_as_versioned”? I seem to have write access to many of the repos, but not this one. nimmo On May 27, 2022, 10:19 PM -0700, nimmo ***@***.***>, wrote: > Joe - I'm going to try the extra splat (forking the repo and telling Gemfile to load my local version). Will report back! > > On May 27, 2022, at 5:45 PM, Joseph D. Cohen ***@***.***> wrote: > > > @nimmolo. Re acts_as_versioned > > I'm also not getting it. > > act_as_versioned line 243: > > has_many :versions, self.version_association_options > > self.version_association_options is a hash > > act_as_versioned lines 160-163: > > self.version_association_options = { > > :class_name => "#{self.to_s}::#{versioned_class_name}", > > :foreign_key => versioned_foreign_key, > > }.merge(options[:association_options] || {}) > > So the call to has_many does use a hash as its last argument. > > But has_many ought to handle that, as it splats the options param: > > # File activerecord/lib/active_record/associations.rb, line 1370 > > def has_many(name, scope = nil, **options, &extension) > > I don't know what to try. Maybe add a second positional arg: > > has_many :versions, nil, self.version_association_options > > Maybe try an extra splat > > has_many :versions, **(self.version_association_options) > > Even worse, I have no idea how to try out these ideas. I forget how I worked with acts_as_versioned when I forked it. > > — > > Reply to this email directly, view it on GitHub, or unsubscribe. > > You are receiving this because you were mentioned.Message ID: ***@***.***> — Reply to this email directly, view it on GitHub <#953 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAALDFHF7YUGTJ26WFU4FGLVMHQWLANCNFSM5XFPEI5Q> . You are receiving this because you were mentioned.Message ID: ***@***.***>

nimmolo added 3 commits May 27, 2022 13:56

Merge branch 'try-rails-6.1' into ruby-2.7

610a608

Change ruby version, delete Gemfile.lock

d58740f

Create Gemfile.lock

a13e939

Refactor do_parse_name_test kwargs

e553f05

This works!

JoeCohen and others added 2 commits May 28, 2022 07:57

Merge branch 'bump-acts_as_versioned' into ruby-2.7

67e5590

Merge branch 'master' into ruby-2.7

e1601f6

nimmolo requested review from mo-nathan, JoeCohen and pellaea May 29, 2022 23:49

nimmolo marked this pull request as draft May 29, 2022 23:53

nimmolo added 2 commits May 29, 2022 17:23

Merge branch 'joe-fix-test-kwargs' into ruby-2.7

f67a19b

Merge branch 'try-to-fix-ipstats-test' into ruby-2.7

e2e41aa

codeclimate bot reviewed May 30, 2022

View reviewed changes

Merge branch 'try-to-fix-ipstats-test' into ruby-2.7

87e562c

codeclimate bot reviewed May 30, 2022

View reviewed changes

test/controllers/account_controller_test.rb Outdated Show resolved Hide resolved

test/controllers/account_controller_test.rb Outdated Show resolved Hide resolved

test/session_extensions.rb Outdated Show resolved Hide resolved

test/session_extensions.rb Outdated Show resolved Hide resolved

Merge branch 'master' into ruby-2.7

008ebfa

nimmolo marked this pull request as ready for review May 30, 2022 09:18

pellaea merged commit 937bb38 into master May 30, 2022

JoeCohen deleted the ruby-2.7 branch June 6, 2022 00:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Ruby 2.7.6 #953

Upgrade to Ruby 2.7.6 #953

nimmolo commented May 27, 2022 •

edited

coveralls commented May 27, 2022 •

edited

nimmolo commented May 27, 2022 •

edited

pellaea commented May 27, 2022 via email

nimmolo commented May 27, 2022

nimmolo commented May 27, 2022 •

edited

pellaea commented May 27, 2022 via email

JoeCohen commented May 27, 2022

pellaea commented May 27, 2022 via email

nimmolo commented May 27, 2022

JoeCohen commented May 27, 2022 via email

nimmolo commented May 27, 2022 via email

nimmolo commented May 28, 2022 via email

JoeCohen commented May 28, 2022

nimmolo commented May 28, 2022 via email

nimmolo commented May 28, 2022 via email

nimmolo commented May 29, 2022

nimmolo commented May 29, 2022

nimmolo commented May 30, 2022

codeclimate bot commented May 30, 2022

nimmolo commented May 30, 2022

JoeCohen commented Oct 11, 2022 via email

Upgrade to Ruby 2.7.6 #953

Upgrade to Ruby 2.7.6 #953

Conversation

nimmolo commented May 27, 2022 • edited

coveralls commented May 27, 2022 • edited

nimmolo commented May 27, 2022 • edited

pellaea commented May 27, 2022 via email

nimmolo commented May 27, 2022

nimmolo commented May 27, 2022 • edited

pellaea commented May 27, 2022 via email

JoeCohen commented May 27, 2022

pellaea commented May 27, 2022 via email

nimmolo commented May 27, 2022

JoeCohen commented May 27, 2022 via email

nimmolo commented May 27, 2022 via email

nimmolo commented May 28, 2022 via email

JoeCohen commented May 28, 2022

nimmolo commented May 28, 2022 via email

nimmolo commented May 28, 2022 via email

nimmolo commented May 29, 2022

nimmolo commented May 29, 2022

nimmolo commented May 30, 2022

codeclimate bot commented May 30, 2022

nimmolo commented May 30, 2022

JoeCohen commented Oct 11, 2022 via email

nimmolo commented May 27, 2022 •

edited

coveralls commented May 27, 2022 •

edited

nimmolo commented May 27, 2022 •

edited

nimmolo commented May 27, 2022 •

edited