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

Allow lib/vmdb/logging.rb to be required without needing Rails autoloading #15376

Merged

Conversation

NickLaMuro
Copy link
Member

tl; dr;

Allows the following to work:

$ bundle exec ruby -I lib -e 'require "manageiq-gems-pending"; require "vmdb/logging"; Vmdb::Loggers.init'

Summary

Through black magic, or some load order/autoloading secret sauce, we are able to include "vmdb/logging", or even "vmdb/logger", without issue when booting our application so that we don't run into errors like undefined constant Loggers for Vmdb and such. But when loading on it's own, it was impossible.

This PR makes some changes to not only make autoloading function, but make the "vmdb/logging" lib less dependent on Rails for it's functionality.

Links

Steps for Testing/QA

  • Tests pass?
  • Can you run the script at the top just fine on this branch? (does it also fail on master?)

When doing the following:

  Dir.glob(File.join(File.dirname(__FILE__), "loggers", "*")).each { |f| require f }

We are including a bunch of classes do the following:

  module Vmdb::Loggers
    class MyLogger < VmdbLogger
    end
  end

This works fine when the module Vmdb::Loggers has been defined, but when
requiring `lib/vmdb/loggers.rb` directly, this is usually not the case.
By pushing the requires of the child loggers to the bottom of the file,
we ensure that the `Vmdb` module has been defined, and it's child,
`Loggers`, has also been defined.

This allows the following:

  require "vmdb/loggers"
  require "vmdb/logging"

To work with minimal prerequisites and doesn't require autoloading from
rails.
@NickLaMuro
Copy link
Member Author

@miq-bot assign @gtanzillo
@miq-bot add_labels refactoring

@gtanzillo I figured I would assign this to you, and you can assign any additional reviewers as you see fit. Hope that makes sense.

Allows us to load vmdb/logging without needed to require
`active_support/dependencies` to included the `delegation` class method
onto modules/objects.

This doesn't save us much space, and reduces the dependency on rails
incase we need to load the logging earlier and possibly without
rails/active_support.

Included some comments to allow greping for the method definitions via a
`git grep "def log\?"` or something similar.
@NickLaMuro NickLaMuro force-pushed the vmdb_logging_without_autoloading branch from 6b546da to 5d9c4ec Compare June 19, 2017 19:30
@miq-bot
Copy link
Member

miq-bot commented Jun 19, 2017

Checked commits NickLaMuro/manageiq@0dfc016~...5d9c4ec with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gtanzillo gtanzillo added this to the Sprint 63 Ending Jun 19, 2017 milestone Jun 19, 2017
@gtanzillo gtanzillo merged commit ed707ba into ManageIQ:master Jun 19, 2017
@NickLaMuro NickLaMuro mentioned this pull request Jun 28, 2017
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants