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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code reference to config.tld_length generates "NameError: uninitialized constant Apartment::Deprecation" #218

Open
mahendrapk opened this issue Mar 2, 2023 · 2 comments

Comments

@mahendrapk
Copy link

mahendrapk commented Mar 2, 2023

Thanks a lot to contributors for bringing this gem back to life. 馃檹

Background

Trying to migrate an existing RoR project that uses influitive/apartment to rails-on-service/apartment

config/initializers/apartment.rb in the existing project had config.tld_length = 1. After switching to ros-apartment the config block started throwing the NameError mentioned in the title.

Steps to reproduce

Apartment.configure do |config|
  //... other settings
  config.tld_length = 1
end

The block throws NameError.

Also when tried in rails console:

image

Expected behavior

Should not throw NameError.

Actual behavior

Throws NameError

System configuration

  • Database: Mysql Ver 8.0.31 for macos12.6 on x86_64 (Homebrew)

  • Apartment version: 2.11.0

  • Apartment config (in config/initializers/apartment.rb or so):

    • use_schemas: false
  • Rails (or ActiveRecord) version: 5.2.0

  • Ruby version: ruby 2.7.1p83 (2020-03-31 revision a0c7c23c9c) [x86_64-darwin20]

@mahendrapk
Copy link
Author

I did some investigation and found out that #72 deprecated the tld_length and introduced below warning:

  Apartment::Deprecation.warn('`config.tld_length` have no effect because it was removed in https://github.com/influitive/apartment/pull/309')

should that also have require 'apartment/deprecation' ?

@mahendrapk
Copy link
Author

mahendrapk commented Mar 2, 2023

For my purpose - I will get rid of tld_length references from the code base as below PRs suggest it has no impact. But feels nice to fix if we can, if all it takes is require.

Ref:

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

No branches or pull requests

1 participant