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

Delayed Job Support #34

Merged
merged 12 commits into from
Mar 23, 2018
Merged

Delayed Job Support #34

merged 12 commits into from
Mar 23, 2018

Conversation

orangewolf
Copy link
Contributor

Hi,
Wanted to see how you felt about adding Delayed::Job support back in to the health monitor. I've added basic queue size support and thought we'd get a PR open before we go any further in developing it.

Totally open to thoughts and feedback.

@coveralls
Copy link

coveralls commented Mar 20, 2018

Coverage Status

Coverage increased (+0.06%) to 99.435% when pulling 76d3321 on notch8:delayed_job into 27abc80 on lbeder:master.

@lbeder
Copy link
Owner

lbeder commented Mar 21, 2018

Thanks for the contribution, @orangewolf! Could you also update the README.md accordingly? In the meantime, let me sync the master branch with the latest version of rubocop (which makes the tests to fail for this PR).

@lbeder
Copy link
Owner

lbeder commented Mar 21, 2018

Could you also fix the following rubocop offenses?

Offenses:
health-monitor-rails.gemspec:34:3: C: Gemspec/OrderedDependencies: Dependencies should be sorted in an alphabetical order within their section of the gemspec. Dependency delayed_job_active_record should appear before sidekiq.
  s.add_development_dependency 'delayed_job_active_record', '>= 4.1'
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/lib/health_monitor/providers/delayed_job_spec.rb:17:1: C: Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body beginning.

@orangewolf
Copy link
Contributor Author

Updated to add readme and fix rubocop warnings

@lbeder
Copy link
Owner

lbeder commented Mar 22, 2018

Can you fix the failing tests? Your PR, unfortunately, still doesn't pass the tests :(

@lbeder
Copy link
Owner

lbeder commented Mar 23, 2018

Well done! 👏👏

I'll merge and release this soon as 8.1.0.

@lbeder lbeder merged commit 309cc2f into lbeder:master Mar 23, 2018
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.

None yet

3 participants