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

Splitting out okcomputer-contrib #56

Open
pbyrne opened this issue Jan 20, 2015 · 7 comments
Open

Splitting out okcomputer-contrib #56

pbyrne opened this issue Jan 20, 2015 · 7 comments
Milestone

Comments

@pbyrne
Copy link
Collaborator

pbyrne commented Jan 20, 2015

We've discussed this in the past, but I'm making an issue for more concentrated conversation. The gem has a lot of tool-specific checks (Resque, Sidekiq, and Delayed Job, and Mongoid) in addition to more generic checks. This raises two questions:

  • Does it make sense to split out the tool-specific checks into okcomputer-contrib (which is also freer to include more and more tool-specific checks and go really hog-wild)? Or maybe even okcomputer-resque and so on?
  • If that makes sense, does it make sense to do this before bumping to 1.0 and get it all done with?
@pbyrne
Copy link
Collaborator Author

pbyrne commented Jan 20, 2015

I'm 70% in favor of splitting out, but almost 0% in favor of doing it before bumping to 1.0, which is basically ready to go now.

@pbyrne
Copy link
Collaborator Author

pbyrne commented Jan 20, 2015

Pinging @anfleene.

@anfleene
Copy link
Contributor

I think we should do it but let's target it for 2.0. I say we bump to 1.0 now and call this current api stable. how does that sound?

@pbyrne
Copy link
Collaborator Author

pbyrne commented Jan 20, 2015

👍

@pbyrne
Copy link
Collaborator Author

pbyrne commented Feb 22, 2016

The discussion in #86 (comment) gave me an idea for an alternate take on this: Rather than split it out into its own gem, keep all the checks included in the main gem, but don't require them by default. This would let us accept more checks into the gem (unless they become such a large and unmanageable list that we feel compelled to split them out) without needing a bunch of if defined? or rescue LoadError guards.

Instead, we could build additional, logically named files to require, like okcomputer/resque or okcomputer/mongoid to allow people to opt into loading checks for those libraries. We might even want to require them manually when we detect the constants.

So… it'd look something like this:

  • Remove all of our if defined? guards in the built-in checks
  • Remove the auto-loading of all built-in checks
    • Load the DB and default checks, though, and the generic checks like SizeThresholdCheck.
  • Make files to load all related checks in one fell swoop, like require "okcomputer/resque", require "okcomputer/delayed_job" and the like.
  • Update the README with these instructions

@anfleene
Copy link
Contributor

That sounds pretty good. That would also be an easy pattern to use in the future if we do split out gems like a contrib or an okcomputer/resque or whatever.

@johnnyshields
Copy link
Contributor

👍 +1 for splitting out okcomputer-contrib and require "okcomputer/resque" etc. No need for individual gems.

@emmahsax emmahsax removed the 2.0 label Sep 25, 2018
@anfleene anfleene added 2.0 Features for 2.0 and removed 2.0 Features for 2.0 labels Jan 9, 2019
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

4 participants