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

Do not use keyword args as last parameter in method calls #735

Merged

Conversation

dneilroth
Copy link
Contributor

@dneilroth dneilroth commented May 25, 2020

Issue: #702

the remaining deprecation warnings in the tests are coming from rspec-mocks and the sqlite3 gem. Fixes for rspec-mocks appear to be in the works here.

there are a ton of gems/ruby-2.7.0/gems/sequel-5.32.0/lib/sequel/adapters/sqlite.rb:114: warning: rb_check_safe_obj will be removed in Ruby 3.0 warnings related to the sqlite3 gem, which is fixed in version '1.4.2', so I think these warnings happen in the tests that use version '1.3.13' but not rails 6. I am not sure tbh, happy to update if anyone has a recommendation.

@mpraglowski would love a review here 🙏

@dneilroth dneilroth marked this pull request as ready for review May 26, 2020 00:48
RAILS_VERSION Outdated
6.0.3
Copy link
Contributor Author

@dneilroth dneilroth May 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of the warnings that are coming from dependencies are solved by bumping to this version of rails

6.0.3
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this solves the many warnings that are coming from rails dependencies https://weblog.rubyonrails.org/2020/5/6/Rails-6-0-3-has-been-released/

@dneilroth dneilroth force-pushed the 2.7_keyword_deprecation_warning branch 3 times, most recently from a23b9ab to 7166bae Compare May 26, 2020 04:26
],
queue: "default"
})
expect(enqueued_jobs[0]).to include(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the enqueued_job now includes a bunch of extra metadata, for example:

'job_class' => 'RailsEventStore::MyAsyncHandler',
'job_id' => random uuid,
'provider_job_id' => nil,
'queue_name' => 'default',
'priority' => nil,
'arguments' => [
  {
    'event_id'        => '83c3187f-84f6-4da7-8206-73af5aca7cc8',
    'data'            => "--- {}\n",
    'metadata'        => "---\n:timestamp: 2019-09-30 00:00:00.000000000 Z\n",
    'event_type'      => 'RubyEventStore::Event',
    '_aj_symbol_keys' => %w[event_id data metadata event_type],
  },
],
'executions' => 0,
'exception_executions' => {},
'locale' => 'en',
'timezone' => 'UTC',
'enqueued_at' => timestamp string,

I am having trouble tracking down exactly why that is getting added now, presumably due to one of the dependency bumps. I changed the test to use include instead of eq so that it will check if those keys + values exist in the job, but it will not check if it is an exact match (since it no longer is).

@dneilroth dneilroth force-pushed the 2.7_keyword_deprecation_warning branch from 7166bae to 1ecc328 Compare May 26, 2020 04:46
@dneilroth dneilroth force-pushed the 2.7_keyword_deprecation_warning branch from 1ecc328 to cc561e8 Compare May 26, 2020 04:46
@fidel fidel merged commit bfe32bf into RailsEventStore:master May 27, 2020
@fidel
Copy link
Contributor

fidel commented May 27, 2020

Thanks for your contribution @dneilroth!

@dneilroth dneilroth deleted the 2.7_keyword_deprecation_warning branch May 28, 2020 02:37
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