Skip to content

Conversation

@steve-sullivan
Copy link
Contributor

No description provided.

Copy link
Member

@anarchivist anarchivist left a comment

Choose a reason for hiding this comment

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

generally looks good; i have a couple of questions. thanks for tackling this!

@@ -0,0 +1,24 @@
# initializers/okcomputer.rb
# Health checks configuration
require Rails.root.join('app/lib/health_checks')
Copy link
Member

Choose a reason for hiding this comment

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

i think you could do the following instead:

Suggested change
require Rails.root.join('app/lib/health_checks')
require 'health_checks'

# Check that DB migrations have run
OkComputer::Registry.register 'database-migrations', OkComputer::ActiveRecordMigrationsCheck.new

# Not sure about this.... seems I need to do this in order to stub in RSPEC
Copy link
Member

Choose a reason for hiding this comment

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

i'm not sure that this is the case. what are you running into if you don't have this environment check for the other tests?

i saw that you needed to stub OkComputer::ActiveRecordCheck.run below, which makes sense - but still, i'm not sure that you need to avoid registering the check if it's running withRAILS_ENV=test.

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.

3 participants