Repository navigation
Parse DISABLE_DELAYED_REQUEUE_BATCHES=false as false - #856
neuromanskie wants to merge 1 commit into
Conversation
Co-authored-by: Patrick Tulskie <patricktulskie@gmail.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Boolean assignment and regression coverage lib/resque/scheduler/configuration.rb, lib/resque/scheduler/env.rb, test/cli_test.rb, test/env_test.rb |
The setter converts non-nil values with to_bool, and environment setup passes values through unchanged. Tests cover environment values, the command-line flag, explicit false, and an absent override. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Possibly related PRs
- resque/resque-scheduler#796: This PR changes the configuration writer and environment assignment introduced by PR
#796.
Suggested reviewers: onyxraven, patricktulskie
Merge Risk: 
79dd7
-
The environment, CLI, explicit boolean, and fallback behaviors are correctly preserved, with no actionable merge risk identified.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: parsing DISABLE_DELAYED_REQUEUE_BATCHES=false as false. Linked Issues check Issue #845is the only active direct link. Its coding requirements are met at the reviewed head.
[#845] Parse environment boolean strings before assignment:
- lib/resque/scheduler/configuration.rb: `…Out of Scope Changes check The changed files are lib/resque/scheduler/configuration.rb, lib/resque/scheduler/env.rb, test/cli_test.rb, and test/env_test.rb. Each change supports #845:
- configuration.rb: the writer now parses s…Full details: Linked Issues check
-
Explanation
Issue
#845is the only active direct link. Its coding requirements are met at the reviewed head.[
#845] Parse environment boolean strings before assignment:- lib/resque/scheduler/configuration.rb:
disable_delayed_requeue_batches=storesnilasnil. For every other value it storesto_bool(value). to_booluses the sameTRUE_VALUESset as the existingverbose,quiet, anddynamicparsing.- Trace for each acceptance criterion:
DISABLE_DELAYED_REQUEUE_BATCHES=false:Cli#optionscopies the string"false".Envassigns it. The writer returnsfalse.DISABLE_DELAYED_REQUEUE_BATCHES=true: the writer returnstrue.- Value-less
--disable-delayed-requeue-batches: OptionParser yieldstrue.to_bool(true)returnstrue. - Explicit boolean
false:to_bool(false)returnsfalse. - Absent option:
options.key?is false, soEnvskips the assignment. The getter falls back toto_bool(environment['DISABLE_DELAYED_REQUEUE_BATCHES']).
[
#845] Do not call the private helper fromEnv:- lib/resque/scheduler/env.rb:
setup_scheduler_configurationassignsoptions[:disable_delayed_requeue_batches]directly. It does not callto_bool, and the!!coercion is removed.
[
#845] Regression coverage:- The change summary reports parameterized tests in test/cli_test.rb for the
'false','true', and flag cases. It also reports a test in test/env_test.rb that checks an explicitfalseoverrides a priortrue. I did not read these test files. The PASS relies on the summary and on the code trace above.
Unverified detail: the change summary says the CLI tests cover the full path. I did not confirm that they exercise
Envas well asCli.No non-coding requirements exist in
#845. The PR#796link is context only. - lib/resque/scheduler/configuration.rb:
Full details: Out of Scope Changes check
-
Explanation
The changed files are lib/resque/scheduler/configuration.rb, lib/resque/scheduler/env.rb, test/cli_test.rb, and test/env_test.rb. Each change supports
#845:- configuration.rb: the writer now parses string and boolean values with
to_bool. This is the required parsing fix. - env.rb: the
!!coercion is removed. The option is passed through, as#845requires. - test/cli_test.rb and test/env_test.rb: these add regression tests for the
#845acceptance criteria.
lib/resque/scheduler/cli.rb is unchanged. The getter fallback is unchanged. No unrelated behavior was found in the reviewed files.
Behavior change to note: an empty
DISABLE_DELAYED_REQUEUE_BATCHESnow parses asfalse. Previously!!treated it as truthy. This follows from the requiredto_boolsemantics and matches the existing getter fallback. - configuration.rb: the writer now parses string and boolean values with
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 4 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Fixes #845.
Envused!!on the raw env string, so"false"disabled batching. Thedisable_delayed_requeue_batches=writer now parses strings with the sameto_boolthe env fallback uses, keeps native booleans, and treatsnilas unset.Envpasses the option through as-is.Made by Claude Opus 5.5 in Claude Code.
Summary by CodeRabbit
trueandfalsevalues from the environment and preserves explicitfalsevalues.