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

Fix warning messages for Rails 6 and Zeitwerk #54

Merged
merged 1 commit into from
Jun 19, 2023
Merged

Conversation

sobrinho
Copy link

@sobrinho sobrinho commented Mar 30, 2023

[Sessions] Sessions won't be captured without a valid release
DEPRECATION WARNING: Initialization autoloaded the constants Scimitar::ServiceProviderConfiguration, Scimitar::Supportable, Scimitar::Filter, Scimitar::Meta, Scimitar::AuthenticationScheme, and Scimitar::EngineConfiguration.

Being able to do this is deprecated. Autoloading during initialization is going
to be an error condition in future versions of Rails.

Reloading does not reboot the application, and therefore code executed during
initialization does not run again. So, if you reload Scimitar::ServiceProviderConfiguration, for example,
the expected changes won't be reflected in that stale Class object.

These autoloaded constants have been unloaded.

Please, check the "Autoloading and Reloading Constants" guide for solutions.

@exterm
Copy link

exterm commented Jun 5, 2023

Can we get this merged @pond?

@pond
Copy link
Member

pond commented Jun 5, 2023

I'll get to this as soon as I can. Sorry for the delay - I was in the UK for 2 weeks (where my family lives - first trip back there for 5 years!) but thanks to lots of people coughing without masks on both of the long haul flights home, I came down with flu the day after I got back. Illness combined with 11 hours of jet lag (to NZ) hit rather hard and I'm only just starting to catch back up on everything now.

@pond
Copy link
Member

pond commented Jun 5, 2023

NB The Rails 7 branch already does this; didn't realise Rails 6 was starting to generate such noise; white space aside (Rails 7 has the code inside to-prepare indented) I see nothing wrong with the solution so should be mergeable.

Copy link
Member

@pond pond left a comment

Choose a reason for hiding this comment

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

Thanks for the submission! Request for indication is given in review comment - the idea is to reduce the number of differences between this and the main Rails 7 branch.

config/initializers/scimitar.rb Show resolved Hide resolved
Copy link
Member

@pond pond left a comment

Choose a reason for hiding this comment

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

Thanks for the submission! Request for indication is given in review comment - the idea is to reduce the number of differences between this and the main Rails 7 branch.

@pond
Copy link
Member

pond commented Jun 19, 2023

Thanks 👍 😄

@pond pond merged commit be7ac73 into RIPAGlobal:v1 Jun 19, 2023
@pond
Copy link
Member

pond commented Jun 19, 2023

This breaks Ruby 2.7 in CI - I guess I must have something wrong in my GitHub configuration given that this wasn't warned about prior to the PR merge.

The issue is that (for some reason on Ruby 2.7 only) an existing application's own config/initializers/scimitar.rb does not have the to-prepare block, but the gem's default does. The only working combination in such a case is either the <= v1.5.2 version without to_prepare, or to add the same modification to the existing application's initializer.

(An existing, unmodified application initializer would be run before one wrapped in to_prepare, so the generic gem's initializer now overwrites the application's data. I would "at a glance" actually expect that in all Ruby versions though tests detecting the issue only fail for Ruby 2.7, for some reason).

This puts us in the very awkward position of introducing a breaking change but being unable to update the major version per semver, because V2 is the Rails 7 gem.

I'm considering options, but v1.5.3 won't be released for now as a result of the above.

pond added a commit that referenced this pull request Sep 16, 2023
pond added a commit that referenced this pull request Sep 16, 2023
…r-54

Backwards-compatible modifications to support #54
@pond
Copy link
Member

pond commented Sep 16, 2023

Very sorry for the passing of so many months. Workload is unrelenting and extraordinary at the moment.

#63 finally "officially" resolves the backwards compatibility issues, merges to v1 and yields v1.5.3, which I just pushed to RubyGems (https://rubygems.org/gems/scimitar/versions/1.5.3). Many thanks for the contribution 😄

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

3 participants