redmine_github_hook icon indicating copy to clipboard operation
redmine_github_hook copied to clipboard

Fix: Redmine 5.0 / Zeitwerk

Open taikii opened this issue 3 years ago • 4 comments

Change to camel case RedmineGithubHook::VERSION -> RedmineGithubHook::Version

taikii avatar Sep 20 '22 15:09 taikii

What issue does this fix? Are there any reproduction steps?

Looking at what Zeitwerk itself is doing, it looks like we're following the exact same pattern: https://github.com/fxn/zeitwerk/blob/main/lib/zeitwerk/version.rb. It is also the pattern used when you generate a new gem using bundler gem something.

I find it quite dubious that this change should be necessary, but will of course reconsider in the face of a reproducible issue.

koppen avatar Sep 21 '22 07:09 koppen

I get an error on startup. We can solve this error by using camelized names. Or it seems that we can customize Inflections. https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#customizing-inflections

=> Booting Puma
=> Rails 6.1.6 application starting in production
=> Run `bin/rails server --help` for more startup options
Exiting
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader/helpers.rb:127:in `const_get': uninitialized constant RedmineGithubHook::Version (NameError)

    parent.const_get(cname, false)
          ^^^^^^^^^^
Did you mean?  RedmineGithubHook::VERSION
      from /usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader/helpers.rb:127:in `cget'
      from /usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader.rb:239:in `block (2 levels) in eager_load'
      from /usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader/helpers.rb:41:in `block in ls'
...

Running bundle exec rake zeitwerk:check does the same.

$ bundle exec rake zeitwerk:check
Hold on, I am eager loading the application.
rake aborted!
NameError: uninitialized constant RedmineGithubHook::Version

    parent.const_get(cname, false)
          ^^^^^^^^^^
Did you mean?  RedmineGithubHook::VERSION
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader/helpers.rb:127:in `const_get'
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader/helpers.rb:127:in `cget'
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader.rb:239:in `block (2 levels) in eager_load'
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader/helpers.rb:41:in `block in ls'
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader/helpers.rb:27:in `each'
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader/helpers.rb:27:in `ls'
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader.rb:234:in `block in eager_load'
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader.rb:219:in `synchronize'
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader.rb:219:in `eager_load'
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader.rb:318:in `each'
/usr/local/bundle/gems/zeitwerk-2.6.0/lib/zeitwerk/loader.rb:318:in `eager_load_all'
/usr/local/bundle/gems/railties-6.1.6/lib/rails/tasks/zeitwerk.rake:11:in `block in <top (required)>'
/usr/local/bundle/gems/railties-6.1.6/lib/rails/tasks/zeitwerk.rake:47:in `block (2 levels) in <top (required)>'
/usr/local/bin/bundle:25:in `load'
/usr/local/bin/bundle:25:in `<main>'
Tasks: TOP => zeitwerk:check
(See full trace by running task with --trace)

taikii avatar Sep 21 '22 21:09 taikii

Wow, that is super surprising, thanks for spotting this.

Looking at Zeitwerks documentation for inner simple constants it seems we should be able to use

module RedmineGithubHook
  VERSION = "3.0.1"
end

without adding a custom inflection rule, but we need to define the constant in lib/redmine_github_hook.rb instead of lib/redmine_github_hook/version.rb.

I'd be all for moving the constant to another file, ditching the version.rb file entirely, but I'd prefer to keep the "classic" all-uppercase constant syntax if possible. Would you be up for changing this PR and see if that solves the problem?

koppen avatar Sep 22 '22 06:09 koppen

RedmineGithubHook::Plugin class is in a separate file because it causes problems when reading the gemspec. Would you please confirm it? So I think it's better to squash the commit because it's dirty.

taikii avatar Sep 24 '22 17:09 taikii

Thanks a lot for catching and fixing this 😀

koppen avatar Oct 03 '22 07:10 koppen