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: set require "administrate/version" #2491

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

JohnnyKei
Copy link
Contributor

We met uninitialized constant Administrate::VERSION error after v0.20.0.

How to reproduce
Remove require "administrate/version" from administrate.gemspec.

bundle exec rspec spec/controllers/admin/application_controller_spec.rb:123

Copy link
Contributor

@dorianmarie dorianmarie left a comment

Choose a reason for hiding this comment

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

Looks good to me, we are not requiring that file except in gemspec https://github.com/search?q=repo%3Athoughtbot%2Fadministrate%20version&type=code

@nickcharlton
Copy link
Member

Which version did you upgrade from? Are you doing anything with the VERSION constant?

FWIW, that line has been in there for 9 years. I'd not expect that to be a problem because of that version because of that: https://github.com/thoughtbot/administrate/blame/main/administrate.gemspec#L3

@sojan-official
Copy link

sojan-official commented Jan 22, 2024

We are also facing this on this pr: chatwoot/chatwoot#8741

here is the trace

Screenshot 2024-01-22 at 4 27 53 PM

I think it throws this error when administrate renders the deprecation warnings.

mostly happening in the views where we use: https://github.com/fishbrain/administrate-field-belongs_to_search

@JohnnyKei
Copy link
Contributor Author

JohnnyKei commented Jan 22, 2024

Which version did you upgrade from? Are you doing anything with the VERSION constant?

FWIW, that line has been in there for 9 years. I'd not expect that to be a problem because of that version because of that: https://github.com/thoughtbot/administrate/blame/main/administrate.gemspec#L3

I did update from 0.19.0 to 0.20.0.

v0.19.0...v0.20.0#diff-3c290ae4fe2e0753529c6c6dfb1ccaa391ac53237bbd0ca7de07b5f60cd68732R43

I think previous version did not use VERSION in the application code, only was used in gemspec.

The example app read from gemspec, so it may work
https://github.com/thoughtbot/administrate/blob/v0.20.0/Gemfile#L4

In thoughtbot#2479, we switched to using a dedicated `ActiveSupport::Deprecation`
instance to handle a deprecation warning. Unfortunately, in some
circumstances, we reference the `VERSION` constant when it's not been
loaded, which seems to happen when we render the deprecation warnings.
@nickcharlton
Copy link
Member

Ah hah, yes, that all adds up. I think this is the right way to go then.

I can't think of a good way to test this, at least with what structure we have already, so I'm not going to let that hold us back.

@nickcharlton nickcharlton merged commit b853f11 into thoughtbot:main Jan 24, 2024
1 check passed
@nickcharlton
Copy link
Member

I've just pushed v0.20.1 which includes this fix, thanks for reporting and providing a solution!

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.

4 participants